Skip to content

fix: flaky test onboarding @no-mmi doesn't make any network requests to infura before onboarding is completed/test-failure-screenshot.png and onboarding @no-mmi Clicks create a new wallet, accepts a secure password, reveals the Secret Recovery Phrase, confirm SRP/test-failure-screenshot.png#24813

Merged
seaona merged 3 commits intodevelopfrom
flaky-fix-stale-elements
May 28, 2024

Conversation

@seaona
Copy link
Copy Markdown
Member

@seaona seaona commented May 28, 2024

Description

This PR fixes more onboarding flakiness onboarding @no-mmi doesn't make any network requests to infura before onboarding is completed/test-failure-screenshot.pn and onboarding @no-mmi Clicks create a new wallet, accepts a secure password, reveals the Secret Recovery Phrase, confirm SRP/test-failure-screenshot.png .

The error is originated when we try to click an element which is in stale state StaleElementReferenceError: stale element reference: stale element not found in the current frame.
This is a race condition originated in the clickElement method, in the following way:

  1. we try to find the clickable element
  2. the element is refreshed in between
  3. right after, we try to click the element from 1 --> this results in the element being stale, as the element we are trying to click is an old instance (1) instead of (2)

Screenshot from 2024-05-28 14-43-54

Extra note: this race condition is surfaced in the Onboarding tests mostly (I've only seen it fail there, so far), since I suspect that the the actions we perform there, all refresh the same react component for onboarding, giving a window for this casuistic to happen

Screenshot from 2024-05-28 14-37-47

Open in GitHub Codespaces

Related issues

Fixes: #24602 (remaining items)

Manual testing steps

  1. Check ci or run onboarding tests multiple times (though I've never been able to repro this locally)

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@github-actions
Copy link
Copy Markdown
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.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [cc36a2b]
Page Load Metrics (728 ± 480 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint621891013115
domContentLoaded9401595
load522582728999480
domInteractive9401595
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@seaona seaona added team-extension-platform Extension Platform team flaky tests labels May 28, 2024
@seaona seaona marked this pull request as ready for review May 28, 2024 13:01
@seaona seaona requested a review from a team as a code owner May 28, 2024 13:01
@seaona seaona added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 28, 2024
@seaona seaona merged commit 70b2258 into develop May 28, 2024
@seaona seaona deleted the flaky-fix-stale-elements branch May 28, 2024 13:38
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 28, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2024
@seaona seaona self-assigned this May 28, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [ca8a454]
Page Load Metrics (1230 ± 613 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint852111203718
domContentLoaded9431484
load72299512301277613
domInteractive9431484
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

error.name === 'StaleElementReferenceError' &&
attempt < retries - 1
) {
await this.driver.delay(1000);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this what was intended?

await this.delay(1000);

I'm not seeing a delay function on the internal webdriver instance. I'm seeing a this.driver.delay is not a function error on various e2e jobs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops that is a great catch! indeed it should be this.delay()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the fix
#24838

thank you v much Mark 🙏

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the bright side, this means that this race-condition was happening quite often 😅

@metamaskbot metamaskbot added release-11.16.6 Issue or pull request that will be included in release 11.16.6 and removed release-11.18.0 labels Jun 4, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

Missing release label release-11.16.6 on PR. Adding release label release-11.16.6 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.16.6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

flaky tests release-11.16.6 Issue or pull request that will be included in release 11.16.6 team-extension-platform Extension Platform team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Flaky Test: Multiple onboarding tests failing due to an error where the "Create a new wallet" button cannot be found.

6 participants