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.

Sign up for GitHub

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

Jump to bottom

test: run WPT files in parallel again #47283

Merged
merged 1 commit into from Mar 31, 2023
Merged

test: run WPT files in parallel again #47283

merged 1 commit into from Mar 31, 2023

Conversation

panva
Copy link
Member

@panva panva commented Mar 28, 2023 •
edited

Allows WPT suites as well as individual WPT files in them to run in parallel.

Refs: #47146
Refs: #47273 (review)

@panva panva mentioned this pull request Mar 28, 2023
Closed
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 28, 2023
@panva

This comment was marked as outdated.

Sign in to view
@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 28, 2023
targos
targos reviewed Mar 28, 2023
View reviewed changes
test/common/wpt.js
Comment on lines +418 to +413
if (queue.length > 0) {
execute(queue.shift());
}
Copy link
Member

Choose a reason for hiding this comment

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

This is not returned or awaited. Is there a risk of unhandled rejection?

Copy link
Member Author

@panva panva Mar 28, 2023 •
edited

Choose a reason for hiding this comment

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

It behaves as expected even with the WPT suite updated (like the daily WPT job does).

This is the await in the queue.

await events.once(worker, 'exit').catch(() => {});

That being said, I am aware it's just a basic concurrency limit function implementation...

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 28, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@panva

This comment was marked as resolved.

Sign in to view
@targos

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot mentioned this pull request Mar 30, 2023
Open
18 tasks
@panva panva added the blocked PRs that are blocked by other issues or PRs. label Mar 30, 2023
@panva

This comment was marked as resolved.

Sign in to view
@panva panva added request-ci Add this label to start a Jenkins CI on a PR. and removed blocked PRs that are blocked by other issues or PRs. labels Mar 30, 2023
@panva
Copy link
Member Author

panva commented Mar 30, 2023 •
edited

Before:

make test-wpt  74.66s user 2.29s system 88% cpu 1:27.07 total

After:

make test-wpt  75.50s user 3.28s system 428% cpu 18.368 total

Ran on an M1 Macbook Pro

> os.availableParallelism()
10

https://ci.nodejs.org/job/node-stress-single-test/391/console

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 30, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50721/

@github-actions github-actions bot mentioned this pull request Mar 31, 2023
Open
34 tasks
@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 31, 2023
@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 31, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 31, 2023
@nodejs-github-bot nodejs-github-bot merged commit c05689e into nodejs:main Mar 31, 2023
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c05689e

@panva panva deleted the try-wpt-files-parallel branch March 31, 2023 17:57
This was referenced Apr 1, 2023
Open
Open
@panva panva added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Apr 2, 2023
@panva
Copy link
Member Author

panva commented Apr 2, 2023

not landing yet because tonight's daily WPT did not generate a report for current-nightly where this was included, will investigate.

@panva panva mentioned this pull request Apr 2, 2023
Closed
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
Merged
@panva panva removed dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Apr 3, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@panva @RafaelGSS
test: run WPT files in parallel again
f756906 
PR-URL: #47283
Refs: #47146
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@panva @RafaelGSS
test: run WPT files in parallel again
ae3c753 
PR-URL: #47283
Refs: #47146
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@panva @RafaelGSS
test: run WPT files in parallel again
b422467 
PR-URL: #47283
Refs: #47146
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
@panva @RafaelGSS
test: run WPT files in parallel again
54261f3 
PR-URL: #47283
Refs: #47146
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@panva panva mentioned this pull request Apr 19, 2023
Closed
@panva panva added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels Apr 19, 2023
This was referenced Apr 19, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonrig anonrig anonrig approved these changes

@targos targos targos approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@panva @nodejs-github-bot @targos @anonrig