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 Cross-platform issue with npm script #397

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

issackjohn
Copy link
Contributor

@issackjohn issackjohn commented Mar 7, 2024

closes #396

This change allows the npm test scripts to also run on windows machines.

The PR:

  • switches the way the browser is defined when running tests. Initially, the browser was determined via environment variables (e.g., BROWSER=chrome node tests/run.mjs). However, this PR alters the approach to use command line arguments instead (e.g., node tests/run.mjs --browser chrome).
  • The Run tests step in the ci workflow now uses the npm scripts
  • make npm run format work as well.

@issackjohn issackjohn marked this pull request as ready for review March 7, 2024 01:09
@bgrins bgrins requested a review from julienw March 7, 2024 02:41
@rniwa rniwa added the trivial change A change that doesn't affect benchmark results label Mar 7, 2024
Copy link
Contributor

@julienw julienw left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for the fix!

@rniwa rniwa requested a review from camillobruni March 7, 2024 09:23
@rniwa rniwa merged commit 8d67f28 into WebKit:main Mar 7, 2024
4 checks passed
webkit-commit-queue pushed a commit to rniwa/WebKit that referenced this pull request Mar 7, 2024
https://bugs.webkit.org/show_bug.cgi?id=270626

Reviewed by Anne van Kesteren.

Merge WebKit/Speedometer#397 into browserbench.org.

* Websites/browserbench.org/Speedometer3.0/package.json:
* Websites/browserbench.org/Speedometer3.0/tests/run.mjs:

Canonical link: https://commits.webkit.org/275782@main
@YahyaMohamedYahya
Copy link

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
trivial change A change that doesn't affect benchmark results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cross-platform issue with npm script for running tests
5 participants