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 expectSyncExitWithoutError() and expectSyncExit() utils #49020

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

test: add expectSyncExitWithoutError() and expectSyncExit() utils

These can be used to check the state and the output of a child
process launched with spawnSync(). They log additional information
about the child process when the check fails to facilitate debugging
test failures.

test: use expectSyncExit{WithErrors} in snapshot tests

..and replace the similar code added for logging.

@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 4, 2023
@joyeecheung
Copy link
Member Author

joyeecheung commented Aug 4, 2023

Many of our tests that launch child processes only check the status of them without additional logging so when they fail, usually what you see from a assert.strictEqual(child.status, 0) is only:

Uncaught AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 0

and without changing the code to do additional logging it's usually impossible to understand exactly what's causing the failure. this is especially troublesome when debugging flaky tests in the CI. So my hope is that we can also gradually use these methods instead of doing assert.strictEqual(child.status, 0) right after launching child processes in the tests....

test/common/README.md Outdated Show resolved Hide resolved
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023
@nodejs-github-bot
Copy link
Collaborator

These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.
..and replace the similar code added for logging.
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 16, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 16, 2023
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

Landed in 6391b3b...a3f12e4

joyeecheung added a commit that referenced this pull request Aug 16, 2023
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: #49020
Reviewed-By: Luigi Pinca <[email protected]>
joyeecheung added a commit that referenced this pull request Aug 16, 2023
..and replace the similar code added for logging.

PR-URL: #49020
Reviewed-By: Luigi Pinca <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: #49020
Reviewed-By: Luigi Pinca <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
..and replace the similar code added for logging.

PR-URL: #49020
Reviewed-By: Luigi Pinca <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
targos pushed a commit that referenced this pull request Nov 27, 2023
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: #49020
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: nodejs/node#49020
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
These can be used to check the state and the output of a child
process launched with `spawnSync()`. They log additional information
about the child process when the check fails to facilitate debugging
test failures.

PR-URL: nodejs/node#49020
Reviewed-By: Luigi Pinca <[email protected]>
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.

4 participants