-
Notifications
You must be signed in to change notification settings - Fork 5.4k
test: fix flaky test " Multichain API Calling wallet_invokeMethod " #37390
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
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. |
✨ Files requiring CODEOWNER review ✨🧪 @MetaMask/qa (1 files, +10 -5)
👨🔧 @MetaMask/wallet-integrations (1 files, +1 -1)
|
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [accd542]
UI Startup Metrics (1244 ± 85 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [c92f5cb]
UI Startup Metrics (1253 ± 77 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
LGTM ! |
seaona
left a comment
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 nice!
wenfix
left a comment
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.
Send it 🚀
Description
The reason for the test failure is that when calling
clickFooterConfirmButtonAndAndWaitForWindowToClose, the window does not close after clicking the confirm button. This behavior is correct, because multiple confirmation windows are open, the next test step checks the display of the next confirmation screen. Therefore, we should not useclickFooterConfirmButtonAndAndWaitForWindowToClose. The fix is to useclickFooterConfirmButtoninstead.Why the test was flaky instead of consistently failing? Because in the test spec, we use
ifconditions to check and confirm the confirmation screens. Since the send calls are triggered at the same time, the order in which the confirmation screens appear can vary. The test verifies the displayed information on confirmation screen regardless the screen order.If the flow goes into a different
ifblock, the test passes; however, if it follows this particularifblock, the test fails.I also improved the function
checkIsSenderAccountDisplayedto usewaitForSelectorto be more stable.I ran this test 10 times and they all passed.
Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Stabilizes multichain E2E by avoiding window-close waits during sequential confirmations and making sender-account checks wait-based.
test/e2e/flask/multichain-api/evm/wallet_invokeMethod.spec.ts)clickFooterConfirmButtonAndAndWaitForWindowToClosewithclickFooterConfirmButtonto proceed without waiting for the window to close when multiple dialogs are pending.test/e2e/page-objects/pages/confirmations/redesign/transaction-confirmation.ts)checkIsSenderAccountDisplayedmore robust by usingwaitForSelectorwith try/catch, returning a boolean after waiting for the sender account element.Written by Cursor Bugbot for commit c92f5cb. This will update automatically on new commits. Configure here.