-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
test: Add dynamic allocation port for ganache, fixtures and test dapp #7356
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7356 +/- ##
=======================================
Coverage 34.59% 34.59%
=======================================
Files 1017 1017
Lines 27150 27150
Branches 2211 2211
=======================================
Hits 9393 9393
Misses 17268 17268
Partials 489 489 ☔ View full report in Codecov by Sentry. |
b4e8f83
to
e1e39f2
Compare
64d87a0
to
2a95782
Compare
5d86ccb
to
6d45502
Compare
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.
LGTM! A couple of comments:
- I've created a ticket so we can link this PR to the issue https://github.com/MetaMask/MetaMask-planning/issues/1393
- About Signature tests, I've created a separate branch for reorg those tests, in a similar fashion as we have on Extension. This hopefully helps reduce e2e time. I will loop back around that topic
7937f77
to
107a160
Compare
Kudos, SonarCloud Quality Gate passed! |
Description
This PR addresses an issue encountered when increasing the number of workers to 3 in the CI pipeline. The problem arose because, when increasing the number of jest workers, Detox creates new emulators in the same VM. However, existing fixtures, Ganache, and the test Dapp were not equipped to dynamically manage port allocation, causing test failures due to port conflicts between workers. These failures were particularly noticeable in confirmation tests, as they utilised Ganache, the test Dapp, and different fixtures for each test.
To resolve this issue, this PR introduces a dynamic port allocation mechanism that is deterministic and relies on
process.pid
instead ofJEST_WORKER_ID
because the worker id is different in the emulator but always1
outside where servers are spun up. This approach ensures that the number of ports is different between the workers, eliminating port clashes and enabling smooth test execution in multi-worker environments.Included all the confirmation tests in the smoke pattern(removed).Manual testing steps
No functional change in the code.
E2E Pipeline
The build average time is 17m.
Before
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/53f3a798-1302-4f91-89f0-c1ca724bee6e
After
without confirmations tests
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6994eecb-bb23-4161-bdac-9a8605334b49
with confirmations tests
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cd363c8a-8f09-4dd0-b822-132488d68852
Related issues
_Fixes https://github.com/MetaMask/MetaMask-planning/issues/1393
Pre-merge author checklist
Pre-merge reviewer checklist