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: Check the toggle for hex data #25899

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Conversation

hjetpoluru
Copy link
Contributor

@hjetpoluru hjetpoluru commented Jul 17, 2024

Description

This PR is to fix the flaky test occurring in the mmi build, where the dynamic menu option 'Portfolio Dashboard' is loaded and hence added the conditional wait only for the mmi build.

Open in GitHub Codespaces

Related issues

Fixes: #24660

Manual testing steps

Run locally or in codespace the test using below command:-
yarn
yarn build:test:mmi
yarn test:e2e:single test/e2e/tests/settings/show-hex-data.spec.js --browser=chrome --debug --leave-running

Note:- Set the variable process.env.MMI = 'true' at the spec file when executing locally.

Validate the CI for mmi should pass.

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.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 17, 2024
@hjetpoluru hjetpoluru marked this pull request as draft July 17, 2024 14:33
@metamaskbot
Copy link
Collaborator

Builds ready [b580504]
Page Load Metrics (265 ± 262 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint761561142211
domContentLoaded1079352010
load511932265546262
domInteractive1079352010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@hjetpoluru hjetpoluru marked this pull request as ready for review July 17, 2024 14:51
@hjetpoluru hjetpoluru requested a review from vthomas13 July 17, 2024 14:53
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.

Only a small Nit.

Comment on lines 35 to 41
if (process.env.MMI && selector === selectors.settingsDiv) {
await driver.waitForSelector(selectors.portfolioMenuOption);
await driver.clickElement(selector);
} else {
await driver.waitForSelector(selector);
await driver.clickElement(selector);
}
Copy link
Contributor

@chloeYue chloeYue Jul 17, 2024

Choose a reason for hiding this comment

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

Suggested change
if (process.env.MMI && selector === selectors.settingsDiv) {
await driver.waitForSelector(selectors.portfolioMenuOption);
await driver.clickElement(selector);
} else {
await driver.waitForSelector(selector);
await driver.clickElement(selector);
}
if (process.env.MMI && selector === selectors.settingsDiv) {
await driver.waitForSelector(selectors.portfolioMenuOption);
}
await driver.waitForSelector(selector);
await driver.clickElement(selector);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chloeYue Thanks for the suggestion! I tried it before but prefer to keep the function(clickElementsInSequence) original pattern. Thanks for understanding.

Copy link
Contributor

Choose a reason for hiding this comment

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

After discussion, we agreed to keep the await driver.waitForSelector(selector); line. Suggested code changed.

vthomas13
vthomas13 previously approved these changes Jul 19, 2024
@hjetpoluru hjetpoluru requested a review from chloeYue July 22, 2024 12:14
Copy link

sonarcloud bot commented Jul 22, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [dedae33]
Page Load Metrics (233 ± 243 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6112696157
domContentLoaded95224105
load411911233506243
domInteractive95224105
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@hjetpoluru hjetpoluru requested a review from vthomas13 July 23, 2024 18:34
@hjetpoluru hjetpoluru merged commit abe6edb into develop Jul 24, 2024
78 of 79 checks passed
@hjetpoluru hjetpoluru deleted the Flaky-test-24660 branch July 24, 2024 13:44
@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
Archived in project
Development

Successfully merging this pull request may close these issues.

flaky test: Check the toggle for hex data Setting the hex data toggle and verify that the textbox appears
5 participants