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: add and use spawnSyncAndExit() and spawnSyncAndExitWithoutError() #49200

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

joyeecheung
Copy link
Member

Replaces expectSyncExit() and expectSyncExitWithoutError(). Since
we usually just check the child process right after its spawned,
these shorthands also takes care of the spawning. This makes the
tests more concise.


I just realized that these shorthands may be more handy in some cases, so added them and switched to these instead (before we start to use them everywhere)

Replaces expectSyncExit() and expectSyncExitWithoutError(). Since
we usually just check the child process right after its spawned,
these shorthands also takes care of the spawning. This makes the
tests more concise.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@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 Aug 16, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

joyeecheung commented Aug 18, 2023

Note to self: switch to this in the SEA test utils, SEA tests have been flaky without useful information in the CI error logs because the sea test utils uses execFileSync() (the tests are probably fine though, judging from some of the logs in nodejs/reliability#640 those are probably caused by having no disk space left on the test machines)

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 18, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 18, 2023
@nodejs-github-bot nodejs-github-bot merged commit 64a5c01 into nodejs:main Aug 18, 2023
27 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 64a5c01

UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
Replaces expectSyncExit() and expectSyncExitWithoutError(). Since
we usually just check the child process right after its spawned,
these shorthands also takes care of the spawning. This makes the
tests more concise.

PR-URL: #49200
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

4 participants