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

[MV3] Fix e2e incremental security test for MV3 test build #16265

Merged
merged 2 commits into from
Oct 27, 2022

Conversation

seaona
Copy link
Contributor

@seaona seaona commented Oct 25, 2022

Explanation

On the mv3 test build, the e2e test for incremental security is failing due to a tiny delay when we render the seed phrase inside the div box.
This fixes the issue by waiting until the element is not empty. This way we make sure the seed phrase is rendered when we make the assertion.

Note: in MV2 this error does not surface as it's slightly faster on rendering the seed phrase. We'll introduce performance metrics for calculating these delays introduced on the MV3 build.

More Information

Screenshots/Screencaps

Before

image

After

image

Manual Testing Steps

  1. Build the mv3 test yarn build:test:mv3
  2. Run the failing test and check it's working yarn test:e2e:single test/e2e/tests/incremental-security.spec.js --browser=chrome

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@seaona seaona self-assigned this Oct 25, 2022
@seaona seaona requested a review from a team as a code owner October 25, 2022 10:33
@seaona seaona requested a review from brad-decker October 25, 2022 10:33
@github-actions
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 [d93b7d8]
Page Load Metrics (2458 ± 115 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint992712369749360
domContentLoaded205828902445238114
load206128902458240115
domInteractive205828902445238114

@@ -130,6 +130,7 @@ describe('Incremental Security', function () {
const revealedSeedPhrase = await driver.findElement(
'.reveal-seed-phrase__secret-words',
);
await driver.waitForNonEmptyElement(revealedSeedPhrase);
Copy link
Contributor

Choose a reason for hiding this comment

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

If we know the seed phrase, can we just use driver.waitForSelector instead, and remove this new method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ty! not in this case, as here we are going to the complete onboarding flow for Create New Wallet, so every time we run it, it's a different seed-phrase

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my bad, I didn't realise it was creating a new wallet. Thanks for getting back to me

@@ -130,6 +130,7 @@ describe('Incremental Security', function () {
const revealedSeedPhrase = await driver.findElement(
'.reveal-seed-phrase__secret-words',
);
await driver.waitForNonEmptyElement(revealedSeedPhrase);
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my bad, I didn't realise it was creating a new wallet. Thanks for getting back to me

@seaona seaona merged commit f03b3a8 into develop Oct 27, 2022
@seaona seaona deleted the e2e-mv3-incremental branch October 27, 2022 12:16
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: [MV3] e2e failure for Incremental Security spec
4 participants