-
Notifications
You must be signed in to change notification settings - Fork 238
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.
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
Refactor e2e tests #425
Refactor e2e tests #425
Conversation
Going to refactor the load test as well so that we do not rely on printing kill commands to terminate the tests anymore. |
{ | ||
"rpc": "http://127.0.0.1:9650/ext/bc/dRTfPJh4jEaRZoGkPc7xreeYbDGBrGWRV48WAYVyUgApsmzGo/rpc", | ||
"chainId": 43214 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did we remove this? This is mentioned in the contract-examples README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was replaced to switch to using environment variables, so that we do not need to overwrite this file in our tests, which seemed pretty jank to me.
This is now handled here: https://github.com/ava-labs/subnet-evm/blob/refactor-e2e-tests/contract-examples/hardhat.config.ts#L6-L7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sweet! We should probably update that specific README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need to cleanup the README. Good callout that I'll need to update the contract-examples README as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the READMEs, thanks for flagging this 🙌
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
|
||
out, err := cmd.CombinedOutput() | ||
fmt.Printf("\nCombined output:\n\n%s\n", string(out)) | ||
gomega.Expect(err).Should(gomega.BeNil()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the ideal place to print an error if there is one. Makes debugging so so much easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combined output:
ExampleTxAllowList
Contract deployed to: 0x52C84043CD9c865236f11d9Fc9F56aa003c1f922
1) should add contract deployer as admin
✓ precompile should see admin address has admin role
✓ precompile should see test address has no role
✓ contract should report test address has no admin role
✓ contract should report admin address has admin role
✓ should not let test address submit txs (57ms)
✓ should not allow noRole to enable itself
✓ should not allow admin to enable noRole without enabling contract (58ms)
✓ should allow admin to add contract as admin (4166ms)
✓ should allow admin to add allowed address as allowed through contract (4107ms)
✓ should let allowed address deploy (4093ms)
✓ should not let allowed add another allowed
✓ should not let allowed to revoke admin
✓ should not let allowed to revoke itself
✓ should let admin to revoke allowed (4063ms)
✓ should not let admin to revoke itself
15 passing (21s)
1 failing
1) ExampleTxAllowList
should add contract deployer as admin:
AssertionError: expected true to be false
+ expected - actual
-true
+false
at Context.<anonymous> (test/tx_allow_list.ts:53:37)
at step (test/tx_allow_list.ts:35:23)
at Object.next (test/tx_allow_list.ts:16:53)
at fulfilled (test/tx_allow_list.ts:7:58)
at processTicksAndRejections (internal/process/task_queues.js:95:5)
[FAILED] Expected
<*exec.ExitError | 0xc000436000>: {
ProcessState: {
pid: 11498,
status: 256,
rusage: {
Utime: {
Sec: 6,
Usec: 90941,
Pad_cgo_0: [0, 0, 0, 0],
},
Stime: {
Sec: 0,
Usec: 704411,
Pad_cgo_0: [0, 0, 0, 0],
},
Maxrss: 279552000,
Ixrss: 0,
Idrss: 0,
Isrss: 0,
Minflt: 96924,
Majflt: 1724,
Nswap: 0,
Inblock: 0,
Oublock: 0,
Msgsnd: 145,
Msgrcv: 145,
Nsignals: 0,
Nvcsw: 1850,
Nivcsw: 7595,
},
},
Stderr: nil,
}
to be nil
In [It] at: /Users/aaronbuchwald/go/src/github.com/ava-labs/subnet-evm/tests/utils/subnet.go:43 @ 01/26/23 15:20:49.916
< Exit [It] tx allow list - /Users/aaronbuchwald/go/src/github.com/ava-labs/subnet-evm/tests/precompile/solidity/suites.go:36 @ 01/26/23 15:20:49.916 (27.514s)
• [FAILED] [27.514 seconds]
CombinedOutput()
will print out stderr if it does run into an error, but gomega will still show the gnarly error from the hardhat command exiting with an error. This does give the line number that it failed on, which is helpful.
Here it is if it also prints the error:
ExampleTxAllowList
Contract deployed to: 0x52C84043CD9c865236f11d9Fc9F56aa003c1f922
1) should add contract deployer as admin
✓ precompile should see admin address has admin role
✓ precompile should see test address has no role
✓ contract should report test address has no admin role
✓ contract should report admin address has admin role
✓ should not let test address submit txs (38ms)
✓ should not allow noRole to enable itself
✓ should not allow admin to enable noRole without enabling contract
✓ should allow admin to add contract as admin (4077ms)
✓ should allow admin to add allowed address as allowed through contract (4057ms)
✓ should let allowed address deploy (4096ms)
✓ should not let allowed add another allowed
✓ should not let allowed to revoke admin
✓ should not let allowed to revoke itself
✓ should let admin to revoke allowed (4081ms)
✓ should not let admin to revoke itself
15 passing (21s)
1 failing
1) ExampleTxAllowList
should add contract deployer as admin:
AssertionError: expected true to be false
+ expected - actual
-true
+false
at Context.<anonymous> (test/tx_allow_list.ts:53:37)
at step (test/tx_allow_list.ts:35:23)
at Object.next (test/tx_allow_list.ts:16:53)
at fulfilled (test/tx_allow_list.ts:7:58)
at processTicksAndRejections (internal/process/task_queues.js:95:5)
Err: exit status 1
[FAILED] Expected
<*exec.ExitError | 0xc000c80040>: {
ProcessState: {
pid: 12185,
status: 256,
rusage: {
Utime: {
Sec: 6,
Usec: 126482,
Pad_cgo_0: [0, 0, 0, 0],
},
Stime: {
Sec: 0,
Usec: 590972,
Pad_cgo_0: [0, 0, 0, 0],
},
Maxrss: 281849856,
Ixrss: 0,
Idrss: 0,
Isrss: 0,
Minflt: 97780,
Majflt: 1434,
Nswap: 0,
Inblock: 0,
Oublock: 0,
Msgsnd: 145,
Msgrcv: 145,
Nsignals: 0,
Nvcsw: 1392,
Nivcsw: 7641,
},
},
Stderr: nil,
}
to be nil
In [It] at: /Users/aaronbuchwald/go/src/github.com/ava-labs/subnet-evm/tests/utils/subnet.go:46 @ 01/26/23 15:24:48.054
< Exit [It] tx allow list - /Users/aaronbuchwald/go/src/github.com/ava-labs/subnet-evm/tests/precompile/solidity/suites.go:36 @ 01/26/23 15:24:48.054 (27.512s)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding it in makes it clearer that the hardhat process failed with exit status 1, but I think the part that was previously confusing was that the hardhat output did not show up.
Why this should be merged
This PR refactors the ginkgo precompile tests to run on a single node network with staking disabled to reduce the complexity in the setup and teardown code.
Running with staking disabled means that it's no longer necessary to start and stop the nodes, since with staking disabled the node will validate all subnets by default.
Therefore, we remove the overhead of restarting the nodes and waiting for them to get healthy a second time.
How this works
This PR updates
./scripts/run.sh
script to switch from launching a network with the avalanche-network-runner to launching a single non-staking node (with a TODO to launch N nodes specified by the caller).The
BeforeSuite
andAfterSuite
functions are modified in the ginkgo tests to call this script and terminate it to ensure that all processes started during testing are cleaned up correctly.This is used for both the precompile tests and the load test each run on its own unique subnet on the same local Avalanche network.
This PR also adds a convenience script
./scripts/run_ginkgo.sh
to compile and run the precompile and load tests in ginkgo.How this was tested
This PR was tested with the new ginkgo tests locally, on an EC2 instance, and in CI.