Skip to content

Conversation

@joyeecheung
Copy link
Member

@joyeecheungjoyeecheung 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

@joyeecheungjoyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@nodejs-github-botnodejs-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
MemberAuthor

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-actionsgithub-actionsbot 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
MemberAuthor

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.

@joyeecheungjoyeecheung mentioned this pull request 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.
@joyeecheungjoyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 22, 2024
@github-actionsgithub-actionsbot 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
MemberAuthor

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
MemberAuthor

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

@joyeecheungjoyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 24, 2024
@nodejs-github-botnodejs-github-bot merged commit a5376c5 into nodejs:mainFeb 24, 2024
@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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
@marco-ippolitomarco-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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
@richardlaurichardlau 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 <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@joyeecheung@nodejs-github-bot@Trott@targos@marco-ippolito