marick / lein-midje

Leiningen plugin for Midje
MIT License
77 stars 29 forks source link

src/leiningen/midje.clj: make-load-facts-form might use error code >125 #60

Open ghost opened 8 years ago

ghost commented 8 years ago

In src/leiningen/midje.clj, make-load-facts-form limits the error code to 255, which is unsafe as it might clash with error codes reserved by the shell - see e.g. man 1 bash:

EXIT STATUS
       The  exit  status  of  an  executed  command is the value returned by the
       waitpid system call or equivalent function.  Exit statuses fall between 0
       and 255, though, as explained below, the shell may use values above 125
       specially.  Exit statuses from shell builtins and compound commands are
       also limited to this range. Under certain circumstances, the shell will use
       special values to indicate specific failure modes.

       For the shell's purposes, a command which exits with a zero exit status
       has succeeded.  An exit status of zero indicates success.  A non-zero exit
       status indicates failure.  When a command terminates on a fatal signal N,
       bash uses the value of 128+N as the exit status.

       If a command is not found, the child process created to execute it returns
       a status of 127.  If a command is found but is not executable, the return
       status is 126.

       If a command fails because of an error during expansion or redirection, the
       exit status is greater than zero.

       Shell builtin commands return a status of 0 (true) if successful, and
       non-zero (false) if an error occurs while they execute.  All builtins return
       an exit status of 2 to indicate incorrect usage.

       Bash itself returns the exit status of the last command executed, unless a
       syntax error occurs, in which case it exits with a non-zero value.  See also
       the exit builtin command below.
ghost commented 8 years ago

Generally it seems advisable to leave some error codes for errors signaled by lein-midje itself, like invalid command line flags, missing config files, or compilation errors.

marick commented 8 years ago

Are you suggesting that (~exit-fn (min 255 failures#))))) should use 124 instead?

ghost commented 8 years ago

Yes, at least. But I think it would be even better if 1 would mean "test failed" and 2,3,4,... would mean "compilation error", "missing config file", "invalid command line flag" and so on. This would allow CI scripts to distinguish a completely broken test / program invocation from a failed test.