Skip to content

Commit ca58ee1

Browse files
authored
fix: flaky test chrome-e2e-api-specs with error NoSuchWindowError: no such window: target window already closed (#37372)
<!-- 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** There is a race condition with windows after personal sign tests is done. <img width="2056" height="1096" alt="image" src="https://github.com/user-attachments/assets/d8081bb0-3dbe-4147-8c3e-01cf6dd12ac4" /> To avoid this, I added checks between actions and it seems to effectively mitigate the failures. I've run this 10 times and success rate is 100% now <img width="588" height="694" alt="image" src="https://github.com/user-attachments/assets/3082d4ae-1042-4e47-b0ad-4dac8a65b525" /> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37372?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** 1. check ci https://github.com/MetaMask/metamask-extension/actions/runs/18934479948/job/54057916990?pr=37372 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds TestDapp-based page/account/network assertions and dapp switching to stabilize the confirmation rejection E2E flow and prevent window errors. > > - **E2E tests (`test/e2e/api-specs/ConfirmationRejectionRule.ts`)**: > - Integrate `TestDapp` to assert page load and verify connected accounts using `DEFAULT_FIXTURE_ACCOUNT_LOWERCASE` before/after actions (connect/cancel/revoke). > - After connecting, validate network switch via `checkNetworkIsConnected('0x539')`. > - Ensure focus switches back to the dapp window after confirmations to avoid window mismatches. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7bbe1dc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent db25338 commit ca58ee1

File tree

1 file changed

+23
-0
lines changed

1 file changed

+23
-0
lines changed

test/e2e/api-specs/ConfirmationRejectionRule.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
} from '@open-rpc/meta-schema';
99
import paramsToObj from '@open-rpc/test-coverage/build/utils/params-to-obj';
1010
import { Driver } from '../webdriver/driver';
11+
import { DEFAULT_FIXTURE_ACCOUNT_LOWERCASE } from '../constants';
1112
import { WINDOW_TITLES, switchToOrOpenDapp } from '../helpers';
13+
import TestDapp from '../page-objects/pages/test-dapp';
1214
import { addToQueue } from './helpers';
1315

1416
type ConfirmationsRejectRuleOptions = {
@@ -54,6 +56,9 @@ export class ConfirmationsRejectRule implements Rule {
5456
params: [{ eth_accounts: {} }],
5557
});
5658

59+
const testDapp = new TestDapp(this.driver);
60+
await testDapp.checkPageIsLoaded();
61+
5762
await this.driver.executeScript(
5863
`window.ethereum.request(${requestPermissionsRequest})`,
5964
);
@@ -84,6 +89,10 @@ export class ConfirmationsRejectRule implements Rule {
8489

8590
await switchToOrOpenDapp(this.driver);
8691

92+
await testDapp.checkConnectedAccounts(
93+
DEFAULT_FIXTURE_ACCOUNT_LOWERCASE,
94+
);
95+
8796
const switchEthereumChainRequest = JSON.stringify({
8897
jsonrpc: '2.0',
8998
method: 'wallet_switchEthereumChain',
@@ -97,6 +106,7 @@ export class ConfirmationsRejectRule implements Rule {
97106
await this.driver.executeScript(
98107
`window.ethereum.request(${switchEthereumChainRequest})`,
99108
);
109+
await testDapp.checkNetworkIsConnected('0x539');
100110
}
101111
} catch (e) {
102112
console.log(e);
@@ -135,6 +145,12 @@ export class ConfirmationsRejectRule implements Rule {
135145
});
136146
// make sure to switch back to the dapp or else the next test will fail on the wrong window
137147
await switchToOrOpenDapp(this.driver);
148+
const testDapp = new TestDapp(this.driver);
149+
await testDapp.checkPageIsLoaded();
150+
await testDapp.checkConnectedAccounts(
151+
DEFAULT_FIXTURE_ACCOUNT_LOWERCASE,
152+
false,
153+
);
138154
} catch (e) {
139155
console.log(e);
140156
}
@@ -203,6 +219,13 @@ export class ConfirmationsRejectRule implements Rule {
203219
await this.driver.executeScript(
204220
`window.ethereum.request(${revokePermissionsRequest})`,
205221
);
222+
223+
const testDapp = new TestDapp(this.driver);
224+
await testDapp.checkPageIsLoaded();
225+
await testDapp.checkConnectedAccounts(
226+
DEFAULT_FIXTURE_ACCOUNT_LOWERCASE,
227+
false,
228+
);
206229
}
207230
} catch (e) {
208231
console.log(e);

0 commit comments

Comments
 (0)