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

fix: Improve flakiness of await driver.findElements in 'importing multiple tokens from search' test #24694

Merged
merged 2 commits into from
May 22, 2024

Conversation

danjm
Copy link
Contributor

@danjm danjm commented May 21, 2024

Description

This improves on a flaky test that I noticed while working to get mv3 tests to pass. I think it is flaky on mv2 as well. The bug is not visually reproducible manually, but I saw it fail twice on CI.

The test clicks "import-tokens-modal-import-button", then waits for for the loading spinner to be not present, and then finds tokens in the list. It finds the token list as a group and then checks that the expected number are present.

The problem is that the list is not fully populated at the exact same time that the loading spinner is removed.

The fix is to rewrite the test so that it waits for the expected number of tokens to be rendered (instead of assuming the token list is immediately in the fully populated state).

Open in GitHub Codespaces

Manual testing steps

This test should continue to pass on all e2e jobs

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.

@danjm danjm requested a review from a team as a code owner May 21, 2024 20:30
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 metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label May 21, 2024
@danjm danjm changed the title Improve flakiness of await driver.findElements in 'importing multiple tokens from search' test fix: Improve flakiness of await driver.findElements in 'importing multiple tokens from search' test May 21, 2024
Copy link

codecov bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.41%. Comparing base (c586d33) to head (f239c7b).
Report is 6 commits behind head on develop.

Current head f239c7b differs from pull request most recent head b4ecf88

Please upload reports for the commit b4ecf88 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #24694   +/-   ##
========================================
  Coverage    67.41%   67.41%           
========================================
  Files         1288     1288           
  Lines        50233    50233           
  Branches     13014    13014           
========================================
  Hits         33863    33863           
  Misses       16370    16370           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [f239c7b]
Page Load Metrics (953 ± 522 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint61161942412
domContentLoaded95515105
load4825549531088522
domInteractive95515105
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link
Contributor

@seaona seaona left a comment

Choose a reason for hiding this comment

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

nice!

@danjm danjm merged commit 0d1dc5e into develop May 22, 2024
71 of 72 checks passed
@danjm danjm deleted the fix-import-tokens-spec branch May 22, 2024 11:26
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [b4ecf88]
Page Load Metrics (1008 ± 559 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6812390189
domContentLoaded9371484
load55305810081165559
domInteractive9371484
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@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
INVALID-PR-TEMPLATE PR's body doesn't match template 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.

4 participants