feat: [E2E] Parallel E2E tests, devstack cmd, and CI workflow#222
feat: [E2E] Parallel E2E tests, devstack cmd, and CI workflow#222
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a standalone devstack management tool and refactors the E2E test suite to support parallel execution. Key improvements include a mutex-protected NewFundedAccount helper to prevent nonce collisions in Anvil, updated Makefile targets for devstack lifecycle management, and the addition of t.Parallel() to numerous tests for increased performance. Furthermore, the TransferService was updated to map specific gRPC errors to forbidden responses. Review feedback suggests implementing a timeout for service discovery to prevent indefinite hangs and improving error handling in the account funding helper to catch configuration issues early.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
=======================================
Coverage ? 32.64%
=======================================
Files ? 124
Lines ? 8658
Branches ? 0
=======================================
Hits ? 2826
Misses ? 5582
Partials ? 250
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
I think first gemini comment should be addressed for sure |
salindne
left a comment
There was a problem hiding this comment.
Looks good, some small comments
|
@sadiq1971 need rebase |
- Add .github/workflows/e2e.yml: single job resolves CANTON_MASTER_KEY once via GITHUB_ENV then runs devstack-up + all three suites + devstack-down - Add /devstack to .gitignore to prevent compiled binary from being tracked
- Write remappings.txt before forge script to override broken foundry.toml remappings (LayerZero paths that don't exist in CI) - Delete foundry.lock to skip lock validation for missing packages - Add || true to forge clean to guard against pre-existing failures - Dump deployer container logs on devstack up failure for diagnosis
The foundry nightly image now runs as a non-root user, which cannot write to the mounted contracts/ethereum-wayfinder volume in CI (owned by the runner's UID). Adding user: root ensures forge can create out/, cache/, and broadcast/ in the mounted directory.
0a60c1d to
f6a13af
Compare
- Add 2-minute timeout to service discovery in DoMain to prevent indefinite hangs when the devstack is unresponsive - Fail fast in NewFundedAccount when tokens > 0 but tokenAddr is zero address, instead of silently skipping the ERC-20 transfer - Add missing trigger paths to e2e workflow (docker-compose.yaml, Makefile, contracts/**, go.mod, go.sum) - Document why the SSH-to-HTTPS git rewrite is required (submodules canton-erc20 and ethereum-wayfinder use SSH URLs)
Summary
t.Parallel()as the first statement; a package-levelanvilFundingMumutex indsl.goserializes Anvil nonce operations so parallelism is safeDSL.NewFundedAccount: new helper that generates a fresh EVM account and funds it with ETH and/or ERC-20 tokens (accepts whole-unitintamounts); used by bridge and indexer tests instead of the sharedAnvilAccount0tests/e2e/cmd/devstack: standalone Go CLI (up/down) that wraps the docker compose lifecycle; Makefile and CI call this instead of rawdocker composecommandspresets.DoMain: docker lifecycle removed;DoMainnow only performs service discovery — the devstack must be running before tests startdevstack-up/devstack-downtargets;CANTON_MASTER_KEYgenerated once at parse time viaexport;test-e2eruns all three suites via existing targets.github/workflows/e2e.yml): resolvesCANTON_MASTER_KEYonce into$GITHUB_ENV, starts devstack, runs api → bridge → indexer, stops devstack unconditionallypkg/transfer/service.gonow maps gRPCINVALID_ARGUMENT/PERMISSION_DENIEDfrom Canton signature verification toForbiddenErrorinstead of HTTP 500Test plan
make devstack-upstarts the stack cleanly withCANTON_MASTER_KEYin envmake test-e2e-api/test-e2e-bridge/test-e2e-indexereach pass individuallymake test-e2eruns all three suites sequentially against one devstackgo test -count=1 ./pkg/transfer/...passes (covers the 403 fix)