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

Bug: Abuse of the exit code #1146

Closed
rakhimov opened this issue Jan 12, 2018 · 5 comments
Closed

Bug: Abuse of the exit code #1146

rakhimov opened this issue Jan 12, 2018 · 5 comments
Milestone

Comments

@rakhimov
Copy link

Description

It is nasty to abuse exit codes in environment where exit codes are specifically for errors.
It doesn't compose or play well with every other command line tools.
If you need to count things, have '--count' flag (e.g, git) that reports to stdout.

It doesn't seem to be documented currently,
but exit codes are limited in range to MaxExitCode (=255).
This is just another smell.

Steps to reproduce

set -e
set -o pipefail

catch_main --list-test-names-only | wc -l

or

# No way to specify memory leak return code. Catch can return 127 as well!
valgrind --tool=memcheck --error-exitcode=127 catch_main

Extra information

  • Catch version: v2.1
  • Operating System: GNU/Linux
  • Compiler+version: GCC, Clang
@horenmar
Copy link
Member

horenmar commented Jan 12, 2018

There are basically three points in this

  1. Returning the number of tests/reporters etc. in return code is weird and unexpected

This one I agree with, but because of backward compatibility is very likely here to stay.

  1. Catch shouldn't use the number of failed tests as return code.

This one I don't agree with. Tests failing is an error. If you care that you cannot tell whether you have XYZ failing tests or are leaking memory, you care about the wrong problem.

  1. Clamping the return code is weird.

Blame POSIX. Both waitid and waitpid only keep lowest 8 bits of process return value, so a return value in the multiple of 256 would lead to a false negative otherwise.

Anyway, 1) and 2) can be easily solved by defining your own main function, that returns different exit codes.

@philsquared
Copy link
Collaborator

FTR I agree with everything @horenmar says. WRT (1) it was a case of "seemed like a good idea at the time".
I don't know if anyone really relies on it - but if they do it's not many. We should probably have changed it in the move to Catch2, but we could do it on the next major version?

And finally I'll add that (3) can also be addressed by providing your own main() - as long as your platform supports the unclamped values (e.g. Windows). I'm not sure how useful it really is in practice, though (if you have more than 255 tests failing do you really care about the exact number?)

@horenmar horenmar closed this as completed Feb 8, 2018
@Bu11etmagnet
Copy link

Returning nonzero when tests fail is very useful (I'd say required) when running tests from a build system.
Let's hope nobody will have 256 (or multiples of it) test failures, because it would look like success.

@philsquared
Copy link
Collaborator

The return value is clamped to 255 - so if 256 tests fail the return value will be 255. It won't look like success.

@philsquared philsquared added this to the 3.0 milestone Feb 15, 2018
@horenmar horenmar removed the Revisit label Jun 16, 2019
lkeegan added a commit to lkeegan/Catch2 that referenced this issue Oct 8, 2020
  - don't warn on zero return code of --list-reporters
  - previously return code was the number of reporters (catchorg#1410, catchorg#1146)
  - as of 2c06ee9 return code is zero on success
@ddomnik
Copy link

ddomnik commented Apr 4, 2023

I agree with the thread starter. For me, I run the tests in a powershell script, which ofc should handle exceptions of other calls. A failed test case is not the same a failed execution error (e.g. calling the catch2 with an invalid argument).

However, as far as I noticed, a script error will always return exit code 255? If so, I have to hope that there is no unit test with 255 test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants