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

test: Enable single test execution across multiple browsers #23357

Merged
merged 27 commits into from
Mar 26, 2024

Conversation

hjetpoluru
Copy link
Contributor

@hjetpoluru hjetpoluru commented Mar 6, 2024

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 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
Copy link
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
Contributor

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
Contributor Author

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
Contributor

HowardBraham commented Mar 7, 2024

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
Collaborator

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
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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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
Contributor

seaona commented Mar 26, 2024

nice addition. LGTM

@metamaskbot
Copy link
Collaborator

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
64 of 66 checks passed
@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
Collaborator

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.
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants