fix: not exit with code 1 when unexpectedly exited during teardown#741
fix: not exit with code 1 when unexpectedly exited during teardown#741
Conversation
✅ Deploy Preview for rstest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the test runner incorrectly exits with code 1 when an unexpected exit occurs during the teardown phase (after tests complete but while closing the worker pool and server). The change distinguishes between exits during test execution versus teardown, treating teardown exits as warnings rather than errors.
- Introduces an
isTeardownflag to track whether teardown has begun - Changes the exit handler to log a warning (yellow) instead of an error (red) during teardown and skips setting
process.exitCode = 1 - Prevents false-positive test failures caused by normal cleanup operations in the test environment
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await pool.close(); | ||
| await closeServer(); | ||
| process.off('exit', unExpectedExit); |
There was a problem hiding this comment.
If pool.close() or closeServer() throws an error, the process.off('exit', unExpectedExit) cleanup won't execute, leaving the event listener attached. This could cause memory leaks or unexpected behavior. Consider wrapping the teardown operations in a try-finally block:
await run();
isTeardown = true;
try {
await pool.close();
await closeServer();
} finally {
process.off('exit', unExpectedExit);
}| await pool.close(); | |
| await closeServer(); | |
| process.off('exit', unExpectedExit); | |
| try { | |
| await pool.close(); | |
| await closeServer(); | |
| } finally { | |
| process.off('exit', unExpectedExit); | |
| } |
| let isTeardown = false; | ||
|
|
||
| const unExpectedExit = (code?: number) => { | ||
| logger.log( | ||
| color.red( | ||
| `Rstest exited unexpectedly with code ${code}, terminating test run.`, | ||
| ), | ||
| ); | ||
| process.exitCode = 1; | ||
| if (isTeardown) { | ||
| logger.log( | ||
| color.yellow( | ||
| `Rstest exited unexpectedly with code ${code}, this is likely caused by test environment teardown.`, | ||
| ), | ||
| ); | ||
| } else { | ||
| logger.log( | ||
| color.red( | ||
| `Rstest exited unexpectedly with code ${code}, terminating test run.`, | ||
| ), | ||
| ); | ||
| process.exitCode = 1; | ||
| } | ||
| }; | ||
| process.on('exit', unExpectedExit); | ||
|
|
||
| await run(); | ||
| isTeardown = true; | ||
| await pool.close(); | ||
| await closeServer(); | ||
| process.off('exit', unExpectedExit); |
There was a problem hiding this comment.
The new teardown exit handling logic lacks test coverage. Consider adding tests to verify:
- That
isTeardown = trueprevents settingprocess.exitCode = 1during teardown - That the exit handler correctly distinguishes between exits before and after teardown begins
- That the exit handler is properly removed after successful teardown
This is important to prevent regressions in the fix for unexpected exit handling during teardown.
Summary
not exit with code 1 when unexpectedly exited during teardown.
Related Links
https://github.com/web-infra-dev/rspack/actions/runs/20021870432/job/57410628304
Checklist