-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
fix: flaky test Snap Account Signatures and Disconnects can connect to the Test Dapp, then #signTypedDataV3, disconnect then connect, then #signTypedDataV4 (async flow approve)
#27887
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. |
Snap Account Signatures and Disconnects can connect to the Test Dapp, then #signTypedDataV3, disconnect then connect, then #signTypedDataV4 (async flow approve)
text: 'Edit', | ||
tag: 'button', | ||
}); | ||
await edit[1].click(); |
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.
since there are 2 elements with the exact same test id and text, we use findClickableElements and then click. Since findClickableElements already makes sure that the element is clickable, we won't run into any race condition where we try to click an element that is not clickable
await driver.waitForSelector({ | ||
css: '[id="chainId"]', | ||
text: '0x539', | ||
}); |
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.
we make sure that we continue to be on localhost in the test dapp, after connecting. There's no reason to change networks.
This will make that the test fails deterministically in the future, if there's ever an unintended network switch
Builds ready [80d2704]
Page Load Metrics (2148 ± 171 ms)
Bundle size diffs
|
Quality Gate passedIssues Measures |
Builds ready [4c888ce]
Page Load Metrics (2066 ± 139 ms)
Bundle size diffs
|
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.
Works when I try it, let's see if it reduces flakiness
Description
After connecting to the test dapp we are automatically switched to Mainnet (this is a recent change, didn't happen before). Then there is a race condition where the network the dapp sees is different from the wallet one, causing a miss-match and making the signature fail never opening the dialog.
This seems an issue on the wallet side, as we should preserve the selected network after connecting, as we used to do before.
I've opened an issue for that here.
This just fixes the issue on the e2e side, but it needs to be fixed on the wallet side and remove the extra step again once the issue is fixed.
Related issues
Fixes: #27892
Manual testing steps
Screenshots/Recordings
See how you are automatically switched to mainnet
switched-to-mainnet.mp4
Pre-merge author checklist
Pre-merge reviewer checklist