-
Notifications
You must be signed in to change notification settings - Fork 5k
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
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
Conversation
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. |
Builds ready [cc36a2b]
Page Load Metrics (728 ± 480 ms)
Bundle size diffs
|
Builds ready [ca8a454]
Page Load Metrics (1230 ± 613 ms)
Bundle size diffs
|
error.name === 'StaleElementReferenceError' && | ||
attempt < retries - 1 | ||
) { | ||
await this.driver.delay(1000); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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()
There was a problem hiding this comment.
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 🙏
There was a problem hiding this comment.
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 😅
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. |
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
andonboarding @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: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
Related issues
Fixes: #24602 (remaining items)
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist