Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restructure tests #93

Merged
merged 20 commits into from
Sep 5, 2022
Merged

Restructure tests #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
54e61e9
remove ERC20token.ts and add assert in catch
qperrot Aug 26, 2022
2c69354
test Starkgate bridge
qperrot Aug 31, 2022
ffb2ec2
use loadContract in test and fixe ERC20 test
qperrot Sep 1, 2022
18b8040
move test to package and use openZeppelin contracts
qperrot Sep 2, 2022
0baa789
fixe name and loadContract_ but not module error yet
qperrot Sep 2, 2022
5c5444f
test for modules utils
qperrot Sep 2, 2022
507bb0c
change name to ERC20.test.ts
qperrot Sep 2, 2022
c30da1b
change link_token.test.ts with new name of ERC20 fill
qperrot Sep 2, 2022
cc380c3
restor to original but still have module not found
qperrot Sep 2, 2022
c249d07
Clean up ERC20 cairo behaviour test
krebernisak Sep 3, 2022
2c1c892
Fix link_token.cairo interface and fix/refactor ERC20 tests
krebernisak Sep 3, 2022
f1759b6
Fix tests and restructure tests runners
krebernisak Sep 3, 2022
7d786ed
Restructure GH workflows
krebernisak Sep 3, 2022
a1bc6a0
Rename workflow/job names
krebernisak Sep 3, 2022
6622de8
Remove postgress service as part of relayer workflow
krebernisak Sep 3, 2022
2d298b6
Lint workflow fix
krebernisak Sep 3, 2022
c0ce6ee
Cleanup NPM dependency tree, use Yarn workspaces more
krebernisak Sep 3, 2022
d821378
Update jest/ts-jest configuration
krebernisak Sep 4, 2022
8ea8fbe
Restructure integration tests + docker env
krebernisak Sep 4, 2022
1db4a8e
Simplify tests - remove 'expectAddressEquality' fn
krebernisak Sep 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .github/workflows/contracts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
name: Contracts

on:
push:
branches:
- develop
- main
pull_request:

name: contracts

jobs:
contract-tests:
name: Contract tests
contracts_run_tests:
name: Run Tests
runs-on: ubuntu-latest
steps:
- name: Checkout sources
Expand All @@ -20,8 +20,8 @@ jobs:
with:
nix_path: nixpkgs=channel:nixos-unstable

- name: Check format
run: nix develop -c make format-cairo-check
- name: Build
run: nix develop -c make build-ts-contracts

- name: Run unit tests
run: nix develop -c ./scripts/test.sh
- name: Test
run: nix develop -c make test-ts-contracts
63 changes: 0 additions & 63 deletions .github/workflows/gauntlet.yml
View file
Open in desktop

This file was deleted.

23 changes: 23 additions & 0 deletions .github/workflows/integration_contracts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Integration Contracts (Vendor, Examples)

on:
push:
branches:
- develop
- main
pull_request:

jobs:
integration_contracts_run_tests:
name: Run Tests
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2
- name: Install Nix
uses: cachix/install-nix-action@d64e0553100205688c0fb2fa16edb0fc8663c590 # v17
with:
nix_path: nixpkgs=channel:nixos-unstable

- name: Test
run: nix develop -c make test-integration-contracts
26 changes: 26 additions & 0 deletions .github/workflows/integration_gauntlet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Integration Gauntlet

on:
push:
branches:
- develop
- main
pull_request:

jobs:
integration_gauntlet_run_tests:
name: Run Tests
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2
- name: Install Nix
uses: cachix/install-nix-action@d64e0553100205688c0fb2fa16edb0fc8663c590 # v17
with:
nix_path: nixpkgs=channel:nixos-unstable

- name: Test
run: nix develop -c make test-integration-gauntlet

- name: Test - Run Gauntlet CLI via Yarn
run: nix develop -c yarn gauntlet
26 changes: 26 additions & 0 deletions .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Lint

on:
push:
branches:
- develop
- main
pull_request:

jobs:
lint_format_check:
name: Format Check
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2
- name: Install Nix
uses: cachix/install-nix-action@d64e0553100205688c0fb2fa16edb0fc8663c590 # v17
with:
nix_path: nixpkgs=channel:nixos-unstable

- name: Install
run: nix develop -c yarn install --frozen-lockfile

- name: Check
run: nix develop -c make format-check
23 changes: 4 additions & 19 deletions .github/workflows/relayer.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: relayer
name: Relayer

on:
push:
Expand All @@ -9,23 +9,8 @@ on:

jobs:
relayer_run_unit_tests:
name: Relayer Run Unit Tests
name: Run Unit Tests
runs-on: ubuntu-latest
env:
DATABASE_URL: postgres://postgres@localhost:5432/postgres?sslmode=disable
services:
postgres:
image: postgres
ports:
- 5432:5432
env:
POSTGRES_HOST_AUTH_METHOD: trust
# Set health checks to wait until postgres has started
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
steps:
- name: Checkout sources
uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # v3.0.2
Expand All @@ -34,8 +19,8 @@ jobs:
with:
nix_path: nixpkgs=channel:nixos-unstable

- name: Compile module
- name: Build
run: nix develop -c make build-go-relayer

- name: Run unit tests
- name: Test
run: nix develop -c make test-unit-go
59 changes: 54 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,21 +79,23 @@ build-ts: build-ts-workspace build-ts-contracts build-ts-examples

.PHONY: build-ts-workspace
build-ts-workspace:
yarn install
yarn install --frozen-lockfile
yarn build

# TODO: use yarn workspaces features instead of managing separately like this
# https://yarnpkg.com/cli/workspaces/foreach
.PHONY: build-ts-contracts
build-ts-contracts:
cd contracts/ && \
yarn install && \
yarn install --frozen-lockfile && \
yarn compile

.PHONY: build-ts-examples
build-ts-examples:
cd examples/contracts/aggregator-consumer && \
yarn install && \
yarn install --frozen-lockfile && \
yarn compile

.PHONY: gowork
gowork:
go work init
Expand Down Expand Up @@ -157,8 +159,55 @@ test-unit-go:
cd ./relayer && go test -v ./... -race -count=10

.PHONY: test-integration
test-integration: test-integration-smoke
test-integration: test-integration-smoke test-integration-contracts test-integration-gauntlet

.PHONY: test-integration-smoke
test-integration-smoke:
ginkgo -v -r --junit-report=tests-smoke-report.xml --keep-going --trace integration-tests/smoke

.PHONY: test-integration-contracts
# TODO: better network lifecycle setup - requires external network (L1 + L2)
test-integration-contracts: build-ts env-devnet-hardhat
cd examples/contracts/aggregator-consumer/ && \
yarn test
cd packages-ts/integration-eqlabs-multisig/ && \
yarn test
cd packages-ts/integration-starkgate/ && \
yarn test

.PHONY: test-integration-gauntlet
# TODO: better network lifecycle setup - tests setup/run their own network (L1 + conflict w/ above if not cleaned up)
test-integration-gauntlet: build-ts env-devnet-hardhat-down
cd packages-ts/starknet-gauntlet/ && \
yarn test
cd packages-ts/starknet-gauntlet-argent/ && \
yarn test
cd packages-ts/starknet-gauntlet-cli/ && \
yarn test
cd packages-ts/starknet-gauntlet-example/ && \
yarn test
cd packages-ts/starknet-gauntlet-multisig/ && \
yarn test
cd packages-ts/starknet-gauntlet-ocr2/ && \
yarn test
cd packages-ts/starknet-gauntlet-oz/ && \
yarn test
cd packages-ts/starknet-gauntlet-starkgate/ && \
yarn test
Comment on lines +180 to +196
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not rely on the toplevel yarn test which uses the Jest config to run all the tests? I personally only use that and ignore the Makefile

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version before this PR just ran Jest tests via root package.json test script.

The problem was not all workspace packages use Jest for testing - our contracts, example project, and 2x integration projects (multisig, starkgate) use hardhat + mocha, so they wouldn't be run.

Then I updated the root package.json test script to iterate over all projects and run their test scripts via "yarn workspaces run test" - but now the problem is Hardhat setup tests expect external network to be started already, and Gauntlet Jest tests run their own network (Integrated Devnet) on beforeAll. This was now a conflict because Integrated Devnet wouldn't start successfully if docker container was already running on the same 5050 port.

So for now I've split the two category of tests as different target, and different CI workflows. Now there is no infra conflict, and all tests are finally ran.

This is enough for this PR I'd say, and the plan for the next one is to restructure how do we run local env in tests as dependencies. Every tests suite should start its own network programatically in test setup, and tear it down when finished. At that point we can just run all tests at once via "yarn workspaces run test" and they would be even more isolated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tldr; I consider this a temp workaround, and will try to remove it as test runs are restructured to be idempotent.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted a GH Issue:


.PHONY: test-ts
test-ts: test-ts-contracts test-integration-contracts test-integration-gauntlet

.PHONY: test-ts-contracts
test-ts-contracts: build-ts-contracts env-devnet-hardhat
cd contracts/ && \
yarn test

# TODO: this script needs to be replaced with a predefined K8s enviroment
.PHONY: env-devnet-hardhat
env-devnet-hardhat:
./ops/scripts/devnet-hardhat.sh

.PHONY: env-devnet-hardhat-down
env-devnet-hardhat-down:
./ops/scripts/devnet-hardhat-down.sh
14 changes: 4 additions & 10 deletions contracts/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,17 @@
"@nomiclabs/hardhat-ethers": "^2.0.5",
"@nomiclabs/hardhat-waffle": "^2.0.3",
"@shardlabs/starknet-hardhat-plugin": "^0.6.2",
"@types/chai": "^4.2.22",
"@types/chai": "^4.3.3",
"@types/elliptic": "^6.4.14",
"@types/mocha": "^9.0.0",
"@types/node": "^18.7.11",
"bn.js": "^5.2.0",
"@types/mocha": "^9.1.1",
"cairo-ls": "^0.0.4",
"chai": "^4.3.4",
"chai": "^4.3.6",
"ethereum-waffle": "^3.4.4",
"ethers": "^5.6.8",
"hardhat": "^2.10.2",
"starknet": "3.9.0",
"ts-node": "^10.4.0",
"typescript": "^4.5.2"
Comment on lines -29 to -30
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these required for tests?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are declared as part of <root>/package.json (just once, single version) and used by the whole yarn workspace. No need to redeclare them here, if a custom version is not required.

"hardhat": "^2.10.2"
},
"dependencies": {
"@chainlink/contracts": "^0.4.2",
"@toruslabs/starkware-crypto": "^1.0.0",
"axios": "^0.24.0"
}
}
12 changes: 8 additions & 4 deletions contracts/src/chainlink/cairo/token/starkgate/presets/link_token.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,15 @@ end

@external
func permissionedMint{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
recipient : felt, amount : Uint256
account : felt, amount : Uint256
):
alloc_locals
permitted_minter_only()
with_attr error_message("LinkToken: no permission"):
permitted_minter_only()
end
local syscall_ptr : felt* = syscall_ptr

ERC20._mint(recipient=recipient, amount=amount)
ERC20._mint(recipient=account, amount=amount)

return ()
end
Expand All @@ -159,7 +161,9 @@ func permissionedBurn{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_ch
account : felt, amount : Uint256
):
alloc_locals
permitted_minter_only()
with_attr error_message("LinkToken: no permission"):
permitted_minter_only()
end
local syscall_ptr : felt* = syscall_ptr

ERC20._burn(account=account, amount=amount)
Expand Down
Loading