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: split wasi tests #51836

Merged
merged 1 commit into from
Feb 24, 2024
Merged

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Feb 22, 2024

Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

Refs: #51822

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/wasi

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@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 Feb 22, 2024
@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 22, 2024

When #51834 lands we need to update the status file to only mark the affected test case as flaky (I suspect it's wasi-poll).

Before the split the wasi tests on my laptop finish in 10s, after it finish in 6s (and it's also the wasi-poll test taking all that 6s while others already finish in ~3s).

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

The CI shows that test-wasi-poll is indeed what triggers the flake: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/25993/

Rebased and updated the status file to only mark test-wasi-poll as flaky.

Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 23, 2024

CI seemed happy with the wasi tests (Windows still failed due to an rm error)

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

Stress tests on Windows for the unmarked tests: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/474/

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2024
@nodejs-github-bot nodejs-github-bot merged commit a5376c5 into nodejs:main Feb 24, 2024
54 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in a5376c5

marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: #51836
Refs: #51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
marco-ippolito pushed a commit that referenced this pull request Feb 26, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: #51836
Refs: #51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
marco-ippolito pushed a commit that referenced this pull request Feb 27, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: #51836
Refs: #51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: #51836
Refs: #51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: #51836
Refs: #51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
Move the child process code into a fixture and split the test
so that it can be run in parallel and it's easier to identify
where the failure is coming from. Also use the
spawnSyncAndExitWithoutError() utility so that the test shows
complete information on failure.

Instead of marking all the wasi tests as flaky, only mark the
wasi-poll one which is flaking in the CI now.

PR-URL: nodejs#51836
Refs: nodejs#51822
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants