Skip to content

test: Enable single test execution across multiple browsers#23357

Merged
hjetpoluru merged 27 commits intodevelopfrom
test/run-single-test-across-browsers
Mar 26, 2024
Merged

test: Enable single test execution across multiple browsers#23357
hjetpoluru merged 27 commits intodevelopfrom
test/run-single-test-across-browsers

Conversation

@hjetpoluru
Copy link
Copy Markdown
Contributor

@hjetpoluru hjetpoluru commented Mar 6, 2024 •
edited
Loading

Description

This PR is to execute single test across multiple browsers(chrome and firefox) locally.
E2E debug is now true by default
Updated ReadMe for the specific changes

Note:- I attempted to convert this to a TypeScript file but was unsuccessful, also it's used internally and hence it should be fine.

Open in GitHub Codespaces

Manual testing steps

Run the test in codespace or locally --> Checkout to the branch
yarn install
yarn build:test
yarn test:e2e:single test/e2e/tests/network/update-network.spec.ts --browser=all --debug --leave-running

Without the browser
yarn test:e2e:single test/e2e/tests/network/update-network.spec.ts

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@hjetpoluru hjetpoluru added team-extension-platform Extension Platform team INVALID-PR-TEMPLATE PR's body doesn't match template labels Mar 6, 2024
@hjetpoluru hjetpoluru self-assigned this Mar 6, 2024
@hjetpoluru hjetpoluru requested a review from a team as a code owner March 6, 2024 21:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 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.

@HowardBraham
Copy link
Copy Markdown
Contributor

HowardBraham commented Mar 6, 2024

A few comments:

  1. You should update .vscode/launch.json to have all as an option, and also test it with that
  2. The name runTestOnSingleBrowser is a little misleading, because it doesn't just run a single test, it runs everything you select. So maybe runTestsOnSingleBrowser?
  3. I think we should make debug be true by default, and I think it makes sense to include that in this PR

@hjetpoluru hjetpoluru added needs-qa Label will automate into QA workspace and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Mar 6, 2024
@hjetpoluru hjetpoluru changed the title Enable single test execution across multiple browsers test: Enable single test execution across multiple browsers Mar 6, 2024
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Mar 6, 2024
@hjetpoluru
Copy link
Copy Markdown
Contributor Author

hjetpoluru commented Mar 6, 2024

Thanks @HowardBraham, I have addressed your comments.

@hjetpoluru hjetpoluru added INVALID-PR-TEMPLATE PR's body doesn't match template and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Mar 6, 2024
@HowardBraham HowardBraham force-pushed the test/run-single-test-across-browsers branch from 141797d to 5b1bc8e Compare March 7, 2024 00:31
@HowardBraham
Copy link
Copy Markdown
Contributor

HowardBraham commented Mar 7, 2024 •
edited
Loading

Sorry, noticed three more things:

  1. The description says Set the browser used; either 'chrome' or 'firefox'., but should also mention 'all'
  2. run-all.js should probably also work with --browser=all. Yeah okay perhaps nobody will ever use this option, but it should be there for parity.
  3. README.md should talk about the 'all' option

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 7, 2024

Builds ready [5b1bc8e]
Page Load Metrics (951 ± 411 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint821751262210
domContentLoaded127136209
load621981951856411
domInteractive127136209
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@chloeYue
Copy link
Copy Markdown
Contributor

chloeYue commented Mar 7, 2024

Not sure if it's the best approach, but could we set the default parameter to "all"? This way, if a user runs an e2e test without specifying a browser, it would automatically execute the tests on both browsers, similar to when we explicitly pass "all" as the parameter.

@hjetpoluru hjetpoluru marked this pull request as draft March 19, 2024 13:55
@hjetpoluru hjetpoluru requested a review from HowardBraham March 20, 2024 19:53
@hjetpoluru hjetpoluru marked this pull request as ready for review March 20, 2024 19:54
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Mar 20, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 20, 2024

Builds ready [7672e5f]
Page Load Metrics (787 ± 451 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint762601213818
domContentLoaded96627168
load642184787940451
domInteractive96527168
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 20, 2024

Builds ready [0303490]
Page Load Metrics (512 ± 407 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint613831216632
domContentLoaded1071302010
load492355512848407
domInteractive1071302010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 25, 2024

Builds ready [7400843]
Page Load Metrics (1159 ± 519 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint711841313216
domContentLoaded11122342512
load54295111591081519
domInteractive11122342512
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 25, 2024

Builds ready [23a9eb9]
Page Load Metrics (1004 ± 599 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint623691588239
domContentLoaded12137433115
load50309110041248599
domInteractive12137433115
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@seaona
Copy link
Copy Markdown
Member

seaona commented Mar 26, 2024

nice addition. LGTM

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 26, 2024

Builds ready [218f6fd]
Page Load Metrics (962 ± 514 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint731641192612
domContentLoaded108627168
load6027449621070514
domInteractive108527168
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@hjetpoluru hjetpoluru merged commit 8a503bc into develop Mar 26, 2024
@hjetpoluru hjetpoluru deleted the test/run-single-test-across-browsers branch March 26, 2024 18:15
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2024
@metamaskbot metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Mar 26, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 26, 2024

Builds ready [17eb408]
Page Load Metrics (914 ± 531 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint773861436632
domContentLoaded2188362010
load6627509141107531
domInteractive2188362010
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@HowardBraham HowardBraham HowardBraham approved these changes

@DDDDDanica DDDDDanica DDDDDanica approved these changes

@seaona seaona seaona approved these changes

@Gudahtt Gudahtt Awaiting requested review from Gudahtt

@danjm danjm Awaiting requested review from danjm

@chloeYue chloeYue Awaiting requested review from chloeYue

@kumavis kumavis Awaiting requested review from kumavis kumavis is a code owner

+1 more reviewer

@brad-decker brad-decker brad-decker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@hjetpoluru hjetpoluru

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-11.15.0 Issue or pull request that will be included in release 11.15.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

@hjetpoluru @HowardBraham @metamaskbot @chloeYue @seaona @brad-decker @DDDDDanica