Skip to content

Commit

Permalink
fix: flaky test `Sentry errors after initialization, after opting int…
Browse files Browse the repository at this point in the history
…o metrics @no-mmi should capture UI application state` (#25137)

## **Description**
This PR fixes the flaky test `Sentry errors after initialization, after
opting into metrics @no-mmi should capture UI application state`. The
logs show how the snapshot assertion does not match with the current
state of the wallet.
Specifically, the network info is failing: the EIP is empty and the
status is unknow, however we expect EIP1559 false and status available.
The problem is that we are taking the snapshot whenever we land at the
login page and the race condition is when the snapshot is taken before
we get the network information from ganache, causing the assertion to
fail.

The solution here is to make sure that the network is ready and the
wallet has it in its state. For that, we perform a loginWithBalance
validation. This will make that we login into the wallet and wait until
we have balance - which results in having the network info in the wallet
state in a deterministic manner now.

The snapshot has also been tweaked, to match the new state (logged in).

```
Expected values to be strictly deep-equal:
+ actual - expected ... Lines skipped

  {
    DNS: 'object',
...
      networksMetadata: {
        networkConfigurationId: {
+         EIPS: {},
+         status: 'unknown'
-         EIPS: {
-           '1559': false
-         },
-         status: 'available'
        }
...
      state: 'CLOSED'
    }
  }
```


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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Check all tests pass in all jobs
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/86270/workflows/970c7910-5e26-4850-a4cf-fbba64d000b6/jobs/3140585

## **Screenshots/Recordings**

![Screenshot from 2024-06-07
11-49-46](https://github.com/MetaMask/metamask-extension/assets/54408225/56b17425-0123-4ec7-9fc1-df21f7b37b08)


## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] 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
seaona authored Jun 7, 2024
1 parent c9e944d commit 47706eb
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
16 changes: 9 additions & 7 deletions test/e2e/tests/metrics/errors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ const { format } = require('prettier');
const { isObject } = require('@metamask/utils');
const { SENTRY_UI_STATE } = require('../../../../app/scripts/lib/setupSentry');
const FixtureBuilder = require('../../fixture-builder');
const { convertToHexValue, withFixtures } = require('../../helpers');
const {
convertToHexValue,
logInWithBalanceValidation,
withFixtures,
} = require('../../helpers');

/**
* Derive a UI state field from a background state field.
Expand Down Expand Up @@ -649,9 +653,8 @@ describe('Sentry errors', function () {
title: this.test.fullTitle(),
testSpecificMock: mockSentryTestError,
},
async ({ driver, mockedEndpoint }) => {
await driver.navigate();
await driver.findElement('#password');
async ({ driver, ganacheServer, mockedEndpoint }) => {
await logInWithBalanceValidation(driver, ganacheServer);

// Trigger error
await driver.executeScript(
Expand Down Expand Up @@ -743,9 +746,8 @@ describe('Sentry errors', function () {
title: this.test.fullTitle(),
testSpecificMock: mockSentryTestError,
},
async ({ driver, mockedEndpoint }) => {
await driver.navigate();
await driver.findElement('#password');
async ({ driver, ganacheServer, mockedEndpoint }) => {
await logInWithBalanceValidation(driver, ganacheServer);

// Trigger error
await driver.executeScript('window.stateHooks.throwTestError()');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@
"showNetworkBanner": true,
"showAccountBanner": true,
"trezorModel": null,
"newPrivacyPolicyToastClickedOrClosed": "object",
"newPrivacyPolicyToastShownDate": "object",
"hadAdvancedGasFeesSetPriorToMigration92_3": false,
"nftsDropdownState": {},
"termsOfUseLastAgreed": "number",
"qrHardware": {},
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"snapsInstallPrivacyWarningShown": true,
"surveyLinkLastClickedOrClosed": "object",
"newPrivacyPolicyToastClickedOrClosed": "object",
"newPrivacyPolicyToastShownDate": "object",
"signatureSecurityAlertResponses": "object",
"switchedNetworkDetails": "object",
"switchedNetworkNeverShowMessage": "boolean",
Expand Down Expand Up @@ -89,9 +89,11 @@
"nonRPCGasFeeApisDisabled": "boolean"
},
"KeyringController": {
"isUnlocked": false,
"isUnlocked": true,
"keyrings": "object",
"vault": "string"
"vault": "string",
"encryptionKey": "string",
"encryptionSalt": "string"
},
"LoggingController": { "logs": "object" },
"MetaMetricsController": {
Expand Down Expand Up @@ -266,9 +268,7 @@
"tokensChainsCache": {},
"preventPollingOnNetworkRestart": false
},
"TokenRatesController": {
"marketData": "object"
},
"TokenRatesController": { "marketData": "object" },
"TokensController": {
"tokens": "object",
"ignoredTokens": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@
"localeMessages": "object",
"metamask": {
"isInitialized": true,
"isUnlocked": false,
"isUnlocked": true,
"isAccountMenuOpen": false,
"isNetworkMenuOpen": false,
"internalAccounts": { "accounts": "object", "selectedAccount": "string" },
"transactions": "object",
"networkConfigurations": "object",
"addressBook": "object",
"marketData": "object",
"confirmationExchangeRates": {},
"pendingTokens": "object",
"customNonceValue": "",
Expand Down Expand Up @@ -74,15 +73,15 @@
"showNetworkBanner": true,
"showAccountBanner": true,
"trezorModel": null,
"newPrivacyPolicyToastClickedOrClosed": "object",
"newPrivacyPolicyToastShownDate": "object",
"hadAdvancedGasFeesSetPriorToMigration92_3": false,
"nftsDropdownState": {},
"termsOfUseLastAgreed": "number",
"qrHardware": {},
"usedNetworks": { "0x1": true, "0x5": true, "0x539": true },
"snapsInstallPrivacyWarningShown": true,
"surveyLinkLastClickedOrClosed": "object",
"newPrivacyPolicyToastClickedOrClosed": "object",
"newPrivacyPolicyToastShownDate": "object",
"signatureSecurityAlertResponses": "object",
"switchedNetworkDetails": "object",
"switchedNetworkNeverShowMessage": "boolean",
Expand All @@ -91,8 +90,8 @@
"previousAppVersion": "",
"previousMigrationVersion": 0,
"currentMigrationVersion": "number",
"selectedNetworkClientId": "string",
"showTokenAutodetectModalOnUpgrade": "object",
"selectedNetworkClientId": "string",
"networksMetadata": {
"networkConfigurationId": {
"EIPS": { "1559": false },
Expand Down Expand Up @@ -205,6 +204,7 @@
"fcmToken": "string",
"accounts": "object",
"accountsByChainId": "object",
"marketData": "object",
"unapprovedDecryptMsgs": "object",
"unapprovedDecryptMsgCount": 0,
"unapprovedEncryptionPublicKeyMsgs": "object",
Expand Down Expand Up @@ -247,7 +247,9 @@
"pendingApprovalCount": "number",
"approvalFlows": "object",
"storageMetadata": {},
"versionFileETag": "string"
"versionFileETag": "string",
"encryptionKey": "string",
"encryptionSalt": "string"
},
"send": "object",
"swaps": "object",
Expand Down

0 comments on commit 47706eb

Please sign in to comment.