-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix "Queued Confirmations Queued Requests Banner Alert Banner is shown on dApp 1, but not on dApp 2 after adding multiple transactions on dApp 1, and one on dApp 2" flaky tests #30029
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
Labels
flaky tests
Sev2-normal
Normal severity; minor loss of service or inconvenience.
type-bug
Something isn't working
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 31, 2025
…nner is shown on dApp 1, but not on dApp 2 after adding transaction on dApp 1, and one on dApp 2 (old confirmation flow)` (#30028) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** The test `Queued Confirmations Queued Requests Banner Alert Banner is shown on dApp 1, but not on dApp 2 after adding transaction on dApp 1, and one on dApp 2 (old confirmation flow)` is flaky and fails with the following error.:  There is one fundamental problem which is that in the test we expect that `switchChain` will trigger a dialog, but that shouldn't be the case, as we already have permissions to that network (comes selected by default, see video below). Then the window management is messed up. Roughly: - Whenever we connect to the dapp2, we don't wait for the dialog window to close - We switch to dapp 1 - We trigger a switchChain request --> here we wait for the dialog to be there. The problem is that the switchChain request don't trigger a dialog (that's expected as that network already has permissions by default) - So now, depending on how fast the dialog from step 1 remains open, will make it to the next step, or not (if the dialog is close fast enough). That's why it doesn't fail all the time (but it should, given the incorrect expectation of waiting for a dialog after the request to switch chains, to an already approved chain) - Then the subsequent actions can work with an old instance of the dialog, but that is closed after some seconds (as it was the Connect dialog which we already accepted) making it fail [](https://codespaces.new/MetaMask/metamask-extension/pull/30028?quickstart=1) ## **Related issues** Fixes: #30012 ## **Manual testing steps** 1. Check [ci run](https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/121745/workflows/e5058e63-1255-4e57-9089-756879fe699f/jobs/4506977/tests) Note: you'll see there's a spec failing but that's a different one that will be tackled in a separate PR, issue [here](#30029) 2. Run test locally `yarn test:e2e:single test/e2e/tests/confirmations/alerts/queued-confirmations.spec.ts --browser=chrome --leave-running=true` ## **Screenshots/Recordings** See how the localhost 7777 is already selected by default. This willl make that any request to switch to that chain won't trigger any dialog (ExpecteD) https://github.com/user-attachments/assets/4bafff15-5282-4362-a626-4fd9360d1ed2 See same behaviour in a prod build https://github.com/user-attachments/assets/e35c5dbb-587c-47ff-be6c-ffa42b5015e9 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
fixed by this PR #30031 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
flaky tests
Sev2-normal
Normal severity; minor loss of service or inconvenience.
type-bug
Something isn't working
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/121471/workflows/b3cb072e-116e-401e-bde4-71a952044a09/jobs/4501688/tests
The text was updated successfully, but these errors were encountered: