Skip to content
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

test: Fix flakiness caused by display of newly switched to network modal #28625

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Nov 21, 2024

Description

This PR fixes the flakiness seen, for example, in https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/111818/workflows/f974462c-8208-4189-8592-928b21f0cfde/jobs/4189413/tests

See this screenshot:

Screenshot from 2024-11-21 16-39-14

The test failure error is: ElementClickInterceptedError: Element <div class="mm-box selectable-list-item-wrapper"> is not clickable at point (866,482) because another element <div class="popover-bg"> obscures it

So the test is attempting to clikc the "Import tokens" button in the opened menu behind the "You are now using Binance" modal, but the Import Tokens button cannot be clicked because the modal intercepts the click

So there is a race condition whereby the test assumes that no modal will interfere with the click, and the test will pass if the click can occur before the modal is rendered, but it will fail the the click is attempted after the modal is rendered.
Prior to the PR in question (#28575) there was no menu, and the "Import tokens" button could be clicked directly. The PR added the menu and move "Import tokens" into that menu, so now the test has to wait for that menu to open before the "Import tokens" button can be clicked, exacerbating the race condition.

That new network modal should not be shown in that scenario

this is the logic that controls whether that network modal should be shown:

    const shouldShowNetworkInfo =
      isUnlocked &&
      currentChainId &&
      !isTestNet &&
      !isSendRoute &&
      !isNetworkUsed &&
      !isCurrentProviderCustom &&
      completedOnboarding &&
      allAccountsOnNetworkAreEmpty &&
      switchedNetworkDetails === null;

(I think that is what folks are above referring to as the "Got it" modal)
Meanwhile, in fixture-builder.js we have:

        usedNetworks: {
          [CHAIN_IDS.MAINNET]: true,
          [CHAIN_IDS.LINEA_MAINNET]: true,
          [CHAIN_IDS.GOERLI]: true,
          [CHAIN_IDS.LOCALHOST]: true,
        },

So for any test that sets the network to something other than those four networks, !isNetworkUsed will evaluate to true, which will result in that modal being shown.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

e2e tests should

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

Copy link
Contributor

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.

@danjm danjm marked this pull request as ready for review November 21, 2024 20:27
@danjm danjm requested a review from a team as a code owner November 21, 2024 20:27
@hjetpoluru hjetpoluru changed the title Fix flakiness caused by display of newly switched to network modal test: Fix flakiness caused by display of newly switched to network modal Nov 21, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [bfa548b]
Page Load Metrics (2044 ± 123 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint172926872044255123
domContentLoaded171626692008245118
load173326842044257123
domInteractive27103542512
backgroundConnect1181312211
firstReactRender623961206632
getState586282411
initialActions01000
loadScripts121621631515224108
setupStore65212136
uiStartup193530242279310149
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@danjm danjm added this pull request to the merge queue Nov 21, 2024
Merged via the queue into develop with commit 3eb37d9 Nov 21, 2024
85 of 87 checks passed
@danjm danjm deleted the fix-flaky-custom-chain-tests branch November 21, 2024 21:47
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-extension-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants