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

Ambiguous cli exit status #2438

Closed
Munter opened this Issue Aug 13, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@Munter
Member

Munter commented Aug 13, 2016

Mocha exits with an exit status equal to the amount of errors in the test run. This is a potentially huge problem causing ambiguous exit codes.

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

This means a test with 256 errors will exit with code 0. Run this example as a proof:

for (var i = 0; i < 256; i += 1) {
    it('test ' + i, function () {
        throw new Error('waat');
    });
}

then echo $?

Further more, certain exit codes have special meaning which can cause tools further down a unix pipe to misbehave. See http://tldp.org/LDP/abs/html/exitcodes.html

Mocha cli should always exit with code 1 if there are tests failing and code 0 if there are no tests failing

@ScottFreeCode

This comment has been minimized.

Member

ScottFreeCode commented Aug 15, 2016

At the very least, clamping the result to a maximum would be necessary to resolve the rollover problem; using 1 for test failure in general seems like a good idea in principle, too, but there are a few things I'd like to double-check.

  • Is anybody actually checking for bash-generated special error results in their JavaScript test commands (tangentially, how portable are they?), or is there any risk that they'll actually be generated in a JavaScript testing scenario and actually cause problems due to expecting Mocha's test failure result?
  • On the flipside, is anybody actually using Mocha's current failure-count return value that we'd need to be aware of when changing this?
  • Is the old behavior documented, such that this would be semver "major"?

(Ok, I know I should be able to look through the documentation again and answer that last one, I'm just throwing it out there in case anybody remembers and can say so before I go looking for it. There's also the fact that it's tested, which in some philosophies is intended partly as documentation, but I don't know how strictly that intersects with semver.)

@boneskull

This comment has been minimized.

Member

boneskull commented Aug 23, 2016

My take on this was that unless there's a reported bug regarding this, let's leave the current behavior as-is for now; except clamp the result as @ScottFreeCode suggested.

Munter added a commit that referenced this issue Aug 23, 2016

@pjmp

This comment has been minimized.

pjmp commented Sep 14, 2016

Mocha cli should always exit with code 1 if there are tests failing and code 0 if there are no tests failing

100% agree with @Munter. At work, i use bash to run mocha tests and if 0 tests are passing, the exit code is 0! That's nuts! Caused a lot of headache for me.

@lucasfcosta

This comment has been minimized.

lucasfcosta commented Sep 14, 2016

@PombaM actually the exit code should be equivalent to how many errors you've had. If you have ran 0 tests, then exit code 0 is ok.
If you have less than 256 failed tests then mocha is supposed to exit with any exit code but 0 and you have just found a new bug.

Regarding this issue: IMO it would be more semantic to exit with 0 when there are no errors and 1 when there are any. According to lots of sources (such as @Munter suggested himself), some codes have meaning, for example, if we exit with 127 then someone may think there may have been an non-existing command somewhere along the way.

Also, I've taken a look at the docs and I couldn't find any information about this kind of behavior.

@boneskull

This comment has been minimized.

Member

boneskull commented Sep 14, 2016

@PombaM What headaches did this cause?

We've established Mocha's behavior is "incorrect"; that's not up for debate.

But we haven't yet established that it's actually causing real-world problems. If it is--and those problems have no reasonable workarounds--then we need to break Mocha's API to fix it. So I'm especially curious what your "headache" was, as it were.

@pjmp

This comment has been minimized.

pjmp commented Sep 15, 2016

Firstly, I am not debating anything, i am just putting forward my two cents here.
Secondly, the "headache" was more of my fault.
I use bash to dynamically get tests files and use Mocha to test our apis. Once I had a situation where i had eg: 50 passing and 2 failing tests and 1 skipped/empty test at the end, the skipped file would exit with exitcode 0. When the test suite ran on the ci, the build would succeed regardless of number of failed tests since the exit code was 0. Not exactly what we would be looking for.

Conclusion:
I say headache was my fault because i had skipped tests in my test suite but if anything to take away from this conversation, sometimes Mocha's exit code can be unreliable.

@boneskull

This comment has been minimized.

Member

boneskull commented Sep 17, 2016

@PombaM That sounds like a different bug--skipped tests causing a false positive. I can't reproduce this trivially--can you open another issue if you can make a minimal, reproducible case?

Munter added a commit that referenced this issue Sep 17, 2016

Munter added a commit that referenced this issue Sep 17, 2016

boneskull added a commit that referenced this issue Sep 18, 2016

Exit process with correct error codes (#2445)
* Exit with code 130 in SIGINT. Refs #2438

* Exit with code 255 if more errors than 255 were returned. Fixes #2438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment