Skip to content

Commit

Permalink
fix: Reduce appInstalledEvent timeout delay (#24695)
Browse files Browse the repository at this point in the history
## **Description**

This improves on a flaky test. I noticed this when working on mv3 e2e
tests, although I believe it is the same problem discussed here
https://github.com/MetaMask/metamask-extension/pull/24016/files#r1580648050

app-installed.spec.js expects an api request to segment. It is possible
for the test to get to the point where it checks for that request before
the request has been made. Decreasing the timeout delay in
background.js, means the request will be called sooner, in time for the
test to correctly assert.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24695?quickstart=1)

## **Manual testing steps**

e2e tests should pass

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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.
  • Loading branch information
danjm committed May 31, 2024
1 parent 86435f6 commit 257fc6a
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 2 deletions.
2 changes: 1 addition & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ const addAppInstalledEvent = () => {
setTimeout(() => {
// If the controller is not set yet, we wait and try to add the "App Installed" event again.
addAppInstalledEvent();
}, 1000);
}, 500);
};

// On first install, open a new tab with MetaMask
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/tests/metrics/app-installed.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
onboardingBeginCreateNewWallet,
onboardingChooseMetametricsOption,
getEventPayloads,
tinyDelayMs,
} = require('../../helpers');
const FixtureBuilder = require('../../fixture-builder');

Expand Down Expand Up @@ -49,7 +50,7 @@ describe('App Installed Events @no-mmi', function () {
},
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
await driver.navigate();

await driver.delay(tinyDelayMs);
await onboardingBeginCreateNewWallet(driver);
await onboardingChooseMetametricsOption(driver, true);

Expand Down

0 comments on commit 257fc6a

Please sign in to comment.