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.

Sign up for GitHub

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

Jump to bottom

🚀 Feature: Add option to exit with standard exit codes #3559

Open
plroebuck opened this issue Nov 8, 2018 · 10 comments · Fixed by zendesk/sunco-mocha#2 · May be fixed by #4989
Open

🚀 Feature: Add option to exit with standard exit codes #3559

plroebuck opened this issue Nov 8, 2018 · 10 comments · Fixed by zendesk/sunco-mocha#2 · May be fixed by #4989
Labels
area: integrations related to working with 3rd party software (e.g., babel, typescript) area: node.js command-line-or-Node.js-specific semver-minor implementation requires increase of "minor" version number; "features" status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal

Comments

@plroebuck
Copy link
Contributor

plroebuck commented Nov 8, 2018 •
edited
Loading

Description

Mocha's exit code is the number of failed tests (up to 255).
This is nonstandard and should be more like Unix/POSIX programs.

process.exitCode should be one of:

  • 0 for success
  • 1 for failure (e.g., internal error, if any tests fail, no tests were run)
  • 2 for failure due to invalid command line usage
  • 126 if "bin/_mocha" not executable (handled by shell)
  • 128+n where n is signal# (e.g., SIGINT=2, so rc=130)

where:

  • 0 is EXIT_SUCCESS (POSIX)
  • 1 is EXIT_FAILURE (POSIX)
  • rest are shell scripting maxims (e.g., sh, ksh, bash) (UNIX)

This should be explicitly added to the documentation.

Notes

Exit codes have a range from 0-255.
An exit value greater than 255 returns an exit code modulo 256.

The 126 exit code can only happen on UNIX and would be provided by the shell itself.

Mocha's current scheme leads to ambiguous error codes.
For example, there is no means to distinguish between 130 test failures and interrupt by <Ctrl>-C.

It was mentioned that this would certain CI systems relied on this exit code abuse.
I fail to see how this could affect them; a test failure from above would still return 1, just not a count.

Related

#2445
#2438 (same problem, but fix only addressed clamping #failures)
@plroebuck plroebuck mentioned this issue Nov 8, 2018
Merged
14 tasks
@boneskull
Copy link
Contributor

boneskull commented Nov 8, 2018 •
edited
Loading

the CI expects the count. I can’t recall which it is. there’s plenty of historical discussion around this.

I’d prefer to err on the side of not breaking stuff for existing consumers rather than causing headaches for potential consumers expecting POSIX codes, but that’s just me..

@plroebuck plroebuck added type: feature enhancement proposal area: usability concerning user experience or interface area: node.js command-line-or-Node.js-specific area: integrations related to working with 3rd party software (e.g., babel, typescript) type: discussion debates, philosophy, navel-gazing, etc. labels Nov 9, 2018
@plroebuck
Copy link
Contributor Author

plroebuck commented Nov 9, 2018 •
edited
Loading

Couldn't find anything specific (outside references to this unknown CI).

Fail to understand the difference one way or another (CI-wise) to exiting with 1 vs. error count -- either way, an "error occurred" state should be triggered. By implication, the CI would do something different depending on the number of errors, which seems nonsensical.

One error is the same as a hundred -- it's a failure.

@plroebuck
Copy link
Contributor Author

@Munter could you add anything?

@plroebuck
Copy link
Contributor Author

I traced this conceptually (exit code=#failures) all the way back to the initial commit (20110828). How and where Mocha invoked process.exit moved around from file to file, but found no references to CIs as the reason -- it was just always done this way.

@boneskull
Copy link
Contributor

read issues mentioning exit codes

@icherkashin icherkashin mentioned this issue Jul 5, 2019
Closed
@blag blag mentioned this issue Apr 10, 2020
Merged
@xirzec xirzec mentioned this issue Mar 17, 2022
Merged
@spiltcoffee spiltcoffee mentioned this issue Nov 24, 2022
Open
@KieranHolroyd KieranHolroyd mentioned this issue Apr 29, 2023
Merged
@73rhodes 73rhodes mentioned this issue May 23, 2023
Merged
2 tasks
@73rhodes 73rhodes linked a pull request May 31, 2023 that will close this issue
Open
2 tasks
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Dec 27, 2023
Closed
@JoshuaKGoldberg JoshuaKGoldberg added the semver-major implementation requires increase of "major" version number; "breaking changes" label Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title Use standard exit codes 🚀 Feature: Use standard exit codes Dec 27, 2023
@JoshuaKGoldberg
Copy link
Member

I was ... surprised, to put it nicely, to learn that Mocha uses the number of failed tests for its status code. What a bizarre behavior. Fixing this to align to POSX conventions would be a breaking change, but a good one.

@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Mocha can use your help with this one! and removed area: usability concerning user experience or interface type: discussion debates, philosophy, navel-gazing, etc. labels Feb 6, 2024
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Mar 4, 2024
Closed
This was referenced Mar 18, 2024
Closed
Merged
@JoshuaKGoldberg JoshuaKGoldberg changed the title 🚀 Feature: Use standard exit codes 🚀 Feature: Add option to exit with standard POSIX exit codes Apr 13, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title 🚀 Feature: Add option to exit with standard POSIX exit codes 🚀 Feature: Add option to exit with standard exit codes Apr 13, 2024
@JoshuaKGoldberg
Copy link
Member

Renaming this issue to make it target adding an option for the new behavior, as written in #4989. We can file a separate followup issue to make the standard exit codes the default behavior in a later major version of Mocha.

@voxpelli
Copy link
Member

I think I would prefer adhering to standard exit code behavior right away

@voxpelli
Copy link
Member

Nevermind, lets get this in now. Would love some references for the:

1 for failure (e.g., internal error, if any tests fail, no tests were run)
2 for failure due to invalid command line usage
126 if "bin/_mocha" not executable (handled by shell)
128+n where n is signal# (e.g., SIGINT=2, so rc=130)

@rantoniuk
Copy link

rantoniuk commented Jun 17, 2024 •
edited
Loading

https://stackoverflow.com/questions/1101957/are-there-any-standard-exit-status-codes-in-linux
http://tldp.org/LDP/abs/html/exitcodes.html (as linked previously in the initial description)

@JoshuaKGoldberg JoshuaKGoldberg added semver-minor implementation requires increase of "minor" version number; "features" and removed semver-major implementation requires increase of "major" version number; "breaking changes" labels Jul 20, 2024
@galargh galargh mentioned this issue Aug 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: integrations related to working with 3rd party software (e.g., babel, typescript) area: node.js command-line-or-Node.js-specific semver-minor implementation requires increase of "minor" version number; "features" status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Milestone
No milestone
5 participants
@voxpelli @rantoniuk @boneskull @JoshuaKGoldberg @plroebuck