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: Flaky test Account Custom Name.. #26062

Merged
merged 11 commits into from
Jul 24, 2024
Merged

Conversation

hjetpoluru
Copy link
Contributor

@hjetpoluru hjetpoluru commented Jul 23, 2024

Description

This PR address the fix flaky test related to the Account Custom Name Persistence persists custom account label through account change and wallet lock

Based on the screenshot from the pipeline, the Create button could not be clicked. Therefore, it has been replaced with the clickElementAndWaitToDisappear function.

https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/92651/workflows/dac61e17-caa2-44ab-979c-05c38b16541b/jobs/3449337/tests

ElementClickInterceptedError: Element <button class="mm-box mm-text mm-button-base mm-button-base--size-sm mm-button-base--ellipsis multichain-account-picker mm-text--body-md-medium mm-text--ellipsis mm-box--padding-0 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-inline-flex mm-box--gap-2 mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-transparent mm-box--rounded-lg"> is not clickable at point (575,112) because another element <label class="mm-box mm-text mm-label mm-label--html-for mm-form-text-field__label mm-text--body-md mm-text--font-weight-medium mm-box--display-inline-flex mm-box--align-items-center mm-box--color-text-default"> obscures it

image

Open in GitHub Codespaces

Related issues

Fixes:
#26061

Manual testing steps

Execute the test locally or codespaces using below commands:
yarn
ENABLE_MV3=false yarn build:test:flask
ENABLE_MV3=false yarn test:e2e:single test/e2e/accounts/create-snap-account.spec.ts --browser=firefox

There should not be CI failures related to the test.

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.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

The PR addresses a flaky test issue by modifying the test script to ensure the 'Create' button is properly clicked and waits for it to disappear.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 23, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

The PR addresses a flaky test issue by modifying the test script to ensure the 'Create' button is properly clicked and waits for it to disappear.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to replace clickElement with clickElementAndWaitToDisappear for the 'Create' button.
  • Removed test-e2e-swap-playwright job from .circleci/config.yml to streamline the CI pipeline.
  • Added SWAP_TEST_ETH_USDC_TRADES_MOCK in test/data/mock-data.js for enhanced swap test scenarios.
  • Simplified logic in test/e2e/api-specs/ConfirmationRejectionRule.ts by removing conditional checks for 'Reject' button.
  • Updated test/e2e/helpers.js to add checks for specific selectors, reducing flaky tests.

24 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

plasmacorral
plasmacorral previously approved these changes Jul 23, 2024
Copy link
Contributor

@plasmacorral plasmacorral left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you!

@plasmacorral plasmacorral dismissed their stale review July 23, 2024 20:01

Missed a lint error

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test issue in the Account Custom Name Persistence test suite by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to replace clickElement with clickElementAndWaitToDisappear for the 'Create' button.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@plasmacorral plasmacorral self-requested a review July 23, 2024 20:05
Copy link
Contributor

@plasmacorral plasmacorral left a comment

Choose a reason for hiding this comment

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

Fixed linting and ran locally on both Firefox and Chrome. LGTM.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test issue in the Account Custom Name Persistence test suite by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to replace clickElement with clickElementAndWaitToDisappear for the 'Create' button.

No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

sonarcloud bot commented Jul 23, 2024

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to replace clickElement with clickElementAndWaitToDisappear for the 'Create' button.
  • Removed 'sendAToken' message key from multiple localization files (app/_locales/*/messages.json), ensuring no references remain in the codebase.
  • Enhanced ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx by adding additional props to the Search component and adjusting padding/margin properties.
  • Introduced textProps to ui/components/multichain/pages/page/components/header/header.tsx for more flexible customization of the Text component.
  • Updated header text and properties in ui/components/multichain/pages/send/send.js to improve UI consistency.

18 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Contributor

@HowardBraham HowardBraham left a comment

Choose a reason for hiding this comment

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

The test failures are not related to this PR

Copy link
Contributor

@chloeYue chloeYue left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the quick fix!

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to replace clickElement with clickElementAndWaitToDisappear for the 'Create' button.
  • Updated app/scripts/background.js to conditionally initialize getSocketBackgroundToMocha during test builds.
  • Added server-mocha-to-background.ts and socket-background-to-mocha.ts for improved communication between Mocha/Selenium tests and the background script.
  • Enhanced test/e2e/helpers.js with a new clickElementSafe helper function to improve test reliability.
  • Updated package.json to include new dependencies and devDependencies for enhanced functionality and compatibility.

71 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/tests/settings/show-hex-data.spec.js to update selectors and add conditional logic for different environments.
  • Updated clickElementsInSequence function to include a conditional check for the MMI environment variable.
  • Improved reliability of the test by ensuring correct elements are interacted with, especially in different environments.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to use clickElementAndWaitToDisappear for the 'Create' button.
  • Refactored development/generate-rc-commits.js to fetch author-team mappings from an external teams.json file, improving maintainability.
  • Added error handling for fetching teams.json and ensured JSON structure correctness.

No major changes found since the last review.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • Modified test/e2e/accounts/account-custom-name.spec.ts to use clickElementAndWaitToDisappear for the 'Create' button.
  • Updated test/e2e/vault-decryption-chrome.spec.js to improve test stability by adding disableServerMochaToBackground and ensuring correct window handling.
  • Replaced Typography with Text component in ui/pages/onboarding-flow/creation-successful/creation-successful.js for design consistency.

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@hjetpoluru
Copy link
Contributor Author

As @HowardBraham mentioned, we will be unable to merge this PR until the other tests are addressed.

Copy link

sonarcloud bot commented Jul 24, 2024

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

This PR addresses a flaky test related to the Account Custom Name Persistence by ensuring the 'Create' button is clicked successfully, even when obscured by other elements.

  • test/e2e/accounts/account-custom-name.spec.ts: Replaced clickElement with clickElementAndWaitToDisappear for the 'Create' button to ensure it is clicked even when obscured.
  • test/e2e/webdriver/chrome.js: Locked ChromeDriver to version 126 to maintain test stability.
  • ui/components/app/import-token/token-list/token-list.component.js: Removed showHalo prop from AvatarToken component.
  • ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx: Removed showHalo prop from AvatarToken component.
  • ui/components/multichain/import-tokens-modal/import-tokens-modal-confirm.js: Removed showHalo prop from AvatarToken component.

5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.67%. Comparing base (335c497) to head (429068f).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26062   +/-   ##
========================================
  Coverage    69.67%   69.67%           
========================================
  Files         1405     1405           
  Lines        49728    49728           
  Branches     13748    13748           
========================================
  Hits         34646    34646           
  Misses       15082    15082           

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

@metamaskbot
Copy link
Collaborator

Builds ready [429068f]
Page Load Metrics (305 ± 254 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint692771235024
domContentLoaded9139382914
load431648305528254
domInteractive9139382914
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@plasmacorral plasmacorral merged commit 83e4b2a into develop Jul 24, 2024
76 checks passed
@plasmacorral plasmacorral deleted the fix-flaky-test-custom-name branch July 24, 2024 22:04
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2024
@metamaskbot metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flaky tests INVALID-PR-TEMPLATE PR's body doesn't match template release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-extension-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants