Skip to content

fix: flaky test Add account should not affect public address when using secret recovery phrase to recover account with non-zero balance @no-mmiaded#27420

Merged
hjetpoluru merged 6 commits intodevelopfrom
flakiness-popup-create-acct
Sep 26, 2024
Merged

fix: flaky test Add account should not affect public address when using secret recovery phrase to recover account with non-zero balance @no-mmiaded#27420
hjetpoluru merged 6 commits intodevelopfrom
flakiness-popup-create-acct

Conversation

@seaona
Copy link
Copy Markdown
Member

@seaona seaona commented Sep 26, 2024 •
edited
Loading

Description

In the process for creating a new account, we get to see this error:

TimeoutError: Waiting for element to be located By(css selector, [data-testid="account-list-menu-details"])
Wait timed out after 10023ms
  (Ran on CircleCI Node 17 of 20, Job test-e2e-chrome)
    at /home/circleci/project/node_modules/selenium-webdriver/lib/webdriver.js:929:17
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

This happens after clicking the account menu, and after clicking the Add account. It seems that the click doesn't hit exactly the button due to the account list loading, making the button move. When the account list is loaded, the click is performed outside the button boundary, and this makes the popup close and you cannot find the next element: "account-list-menu-details" element.

I haven't been able to reproduce this but I've seen this behaviour in the manual process

Open in GitHub Codespaces

Related issues

Fixes: those 3 are fixed in this PR, as they were caused by the same issue

Manual testing steps

  1. Check ci
  2. Run tests locally

Screenshots/Recordings

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.

@seaona seaona requested a review from a team as a code owner September 26, 2024 11:18
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 26, 2024

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.

@seaona seaona self-assigned this Sep 26, 2024
@github-actions github-actions bot added the team-extension-platform Extension Platform team label Sep 26, 2024
@seaona seaona added the area-qa Relating to QA work (Quality Assurance) label Sep 26, 2024
chloeYue
chloeYue previously approved these changes Sep 26, 2024
View reviewed changes
Copy link
Copy Markdown
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 ! I have already migrated account list in page object model, i will try to convert this test to use that in separate PR

@hjetpoluru hjetpoluru self-requested a review September 26, 2024 12:41
hjetpoluru
hjetpoluru previously approved these changes Sep 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@hjetpoluru hjetpoluru left a comment

Choose a reason for hiding this comment

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

LGTM

@hjetpoluru hjetpoluru self-requested a review September 26, 2024 12:52
@hjetpoluru
Copy link
Copy Markdown
Contributor

hjetpoluru commented Sep 26, 2024 •
edited
Loading

@seaona - I notice this failure and we could implement the similar here too or is it different issue. I am thinking you're still working on this flaky test and hence I reverted my approval.

HowardBraham
HowardBraham previously approved these changes Sep 26, 2024
View reviewed changes
@seaona seaona dismissed stale reviews from hjetpoluru, chloeYue, and HowardBraham via b15a28d September 26, 2024 17:19
@seaona
Copy link
Copy Markdown
Member Author

seaona commented Sep 26, 2024

@seaona - I notice this failure and we could implement the similar here too or is it different issue. I am thinking you're still working on this flaky test and hence I reverted my approval.

hey @hjetpoluru nice catch! I've added the fix to a couple of more testcases!
And thank you @chloeYue !
When we implement page objects for this, we could add this condition into the page object itself, so any test utilizing the page object will already mitigate the flakiness, and it won't happen again 👍

HowardBraham
HowardBraham previously approved these changes Sep 26, 2024
View reviewed changes
@davidmurdoch
Copy link
Copy Markdown
Contributor

davidmurdoch commented Sep 26, 2024

A super nit here, please ignore if you want.

Should this use driver.waitForSelector instead of driver.findElement? The end result is the same, but I think the use case here is perfect for driver.waitForSelector (the two function do literally the exact the same thing under the hood, so it really doesn't matter.)

@seaona
Copy link
Copy Markdown
Member Author

seaona commented Sep 26, 2024

A super nit here, please ignore if you want.
Should this use driver.waitForSelector instead of driver.findElement? The end result is the same, but I think the use case here is perfect for driver.waitForSelector (the two function do literally the exact the same thing under the hood, so it really doesn't matter.)

yeah, that's a good point @davidmurdoch 😍 I've updated it, thank you 🙏

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Sep 26, 2024

Builds ready [42eb8bb]
Page Load Metrics (1709 ± 81 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40020771649336161
domContentLoaded14662023168715876
load14742086170916881
domInteractive1310134199
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Sep 26, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@hjetpoluru
Copy link
Copy Markdown
Contributor

hjetpoluru commented Sep 26, 2024

@seaona, looks the test failing in CI and something strange from the logs
Screenshot 2024-09-26 at 4 52 23 PM
I will rerun the test again to see if it occurs.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Sep 26, 2024

Builds ready [d5ac079]
Page Load Metrics (1700 ± 119 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint146024921694249120
domContentLoaded145223961672234113
load146224951700248119
domInteractive1996412210
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@hjetpoluru hjetpoluru merged commit 7a5e4d6 into develop Sep 26, 2024
@hjetpoluru hjetpoluru deleted the flakiness-popup-create-acct branch September 26, 2024 22:01
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2024
@metamaskbot metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@davidmurdoch davidmurdoch davidmurdoch approved these changes

@DDDDDanica DDDDDanica DDDDDanica approved these changes

@HowardBraham HowardBraham HowardBraham left review comments

@hjetpoluru hjetpoluru Awaiting requested review from hjetpoluru

@chloeYue chloeYue Awaiting requested review from chloeYue

Assignees

@seaona seaona

Labels

area-qa Relating to QA work (Quality Assurance) flaky tests release-12.6.0 Issue or pull request that will be included in release 12.6.0 team-extension-platform Extension Platform team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@seaona @hjetpoluru @davidmurdoch @metamaskbot @HowardBraham @DDDDDanica @chloeYue