Skip to content

Commit

Permalink
fix: only set error codes when they are non zero (#7363)
Browse files Browse the repository at this point in the history
* fix: only set error codes when they are non zero

I'm working on an internal CLI that runs jest, eslint, and sasslint all in one single process for our frontend team to run from their `package.json`'s

Aka something like, `"test": "cli test"`.

The code `process.on('exit', () => (process.exitCode = code));` adds a listener that is overriding the exit codes we're setting so that when jest is successful, and eslint or sasslint fail, I can't exit with a failing code.

Therefore our CI's are reporting as passed even though the linting fails.

It seems as though there's no reason to actually set the error code at all unless it's non-zero?

Let me know what the thoughts here are. I can come back in and add more, add some additional tests, etc if this looks like an ok change.

Thanks!

* fix: check if error code is a non-zero number

* chore: update changelog

* Update CHANGELOG.md

* fix: remove whitespace

* move the check inside the process exit handler

* add a breaking change notification
  • Loading branch information
jcreamer898 authored and rickhanlonii committed Nov 21, 2018
1 parent 4b9082e commit b3c10cb
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Features

- `[jest-cli]` [**BREAKING**] Only set error process error codes when they are non-zero ([#7363](https://github.com/facebook/jest/pull/7363))
- `[jest-validate]` Add support for comments in `package.json` using a `"//"` key [#7295](https://github.com/facebook/jest/pull/7295))
- `[jest-config]` Add shorthand for watch plugins and runners ([#7213](https://github.com/facebook/jest/pull/7213))
- `[jest-config]` [**BREAKING**] Deprecate `setupTestFrameworkScriptFile` in favor of new `setupFilesAfterEnv` ([#7119](https://github.com/facebook/jest/pull/7119))
Expand Down
7 changes: 6 additions & 1 deletion packages/jest-cli/src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,12 @@ const readResultsAndExit = (
) => {
const code = !result || result.success ? 0 : globalConfig.testFailureExitCode;

process.on('exit', () => (process.exitCode = code));
// Only exit if needed
process.on('exit', () => {
if (typeof code === 'number' && code !== 0) {
process.exitCode = code;
}
});

if (globalConfig.forceExit) {
if (!globalConfig.detectOpenHandles) {
Expand Down

0 comments on commit b3c10cb

Please sign in to comment.