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 flaky test "lockdown the UI and background environments are locked down" #24812

Merged
merged 1 commit into from
May 28, 2024

Conversation

chloeYue
Copy link
Contributor

@chloeYue chloeYue commented May 28, 2024

Description

This PR addresses the flaky 'lockdown' e2e test, specifically when running in Firefox, which is a tricky one.

Root Cause: The flakiness is caused by the rapid execution of the lockdown test script in Firefox immediately after navigating to the BACKGROUND page. This quick transition did not allow sufficient time for the environment to stabilize, preventing the script from accurately assessing the lockdown state.

Fix Implemented: To mitigate this issue, I introduced a await driver.delay(1000) after navigating to the BACKGROUND page and before executing the lockdown test script. This brief pause ensures that the environment is fully loaded and stable, allowing the script to run under consistent conditions and accurately verify the lockdown state.

While I'm agree that the use of delay should be minimized, this test's specific nature and the absence of a way to use the driver to wait for an element's status makes it necessary to use the delay() method.

Results:
Before the Fix: The flakiness could be reproduced locally on Firefox with approximately a 30% occurrence rate.
After the Fix: After implementing the changes, I ran the test 20 consecutive times without encountering any flakiness, demonstrating the effectiveness of the fix.

Open in GitHub Codespaces

Related issues

Fixes: #24621

Manual testing steps

  1. Run the test several times yarn test:e2e:single test/e2e/tests/request-queuing/ui.spec.js --browser=firefox --leave-running --retryUntilFailure --retries=10
  2. Check ci jobs

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.

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.

@metamaskbot
Copy link
Collaborator

Builds ready [7a2ff9e]
Page Load Metrics (1360 ± 618 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint762351354622
domContentLoaded987202110
load64343213601288618
domInteractive987202110
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@chloeYue chloeYue merged commit 1fe471d into develop May 28, 2024
115 of 117 checks passed
@chloeYue chloeYue deleted the chloe-fix-24621 branch May 28, 2024 11:56
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2024
@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
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Flaky Test: "lockdown the UI and background environments are locked down"
4 participants