Re: [Problem] test_must_fail makes possibly questionable assumptions about exit_code.

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 28, 2018, 00:16 UTC
Message-ID: <20180228001616.GJ174036@aiede.svl.corp.google.com>
In-Reply-To: <005501d3b025$c0057ce0$401076a0$@nexbridge.com>

Hi,

Randall S. Becker wrote:

Show 19 quoted lines

> After months of arguing with some platform developers on this subject, the
> perl spec was held over my head repeatedly about a few lines that are
> causing issues. The basic problem is this line (test-lib-functions.sh, line
> 633, still in ffa952497)
>
>>        elif test $exit_code -gt 129 && test $exit_code -le 192
>>       then
>>               echo >&2 "test_must_fail: died by signal $(($exit_code - 128)):
>
> According to the perl spec http://perldoc.perl.org/functions/die.html, die
> basically takes whatever errno is, mods it with 256 and there you go. EBADF
> is what is used when perl reads from stdin and calls die - that's standard
> perl. In most systems, you end up with something useful, when EBADF is 9.
> But when it is 4009, you get a garbage answer (4009 mod 256 a.k.a. 169).
> However, only 128-165 are technically reserved for signals, rather than all
> the way up to 192, which may be true in some places but not everywhere.
>
> The advice (I'm putting that nicely) I received was to use exit so that the
> result is predictable - unlikely to be useful in the 15K test suites in git.

The fundamental thing is the actual Git commands, not the tests in the testsuite, no?

In the rest of git, die() makes a command exit with status 128. The trouble here is that our code in Perl is assuming the same meaning for die() but using perl's die builtin instead. That suggests a few options:

 a) We could override the meaning of die() in Git.pm.  This feels
    ugly but if it works, it would be a very small patch.

 b) We could forbid use of die() and use some git_die() instead (but
    with a better name) for our own error handling.

 c) We could have a special different exit code convention for
    commands written in Perl.  And then change expectations whenever a
    command is rewritten in C.  As you might expect, I don't like this
    option.

 d) We could wrap each command in an eval {...} block to convert the
    result from die() to exit 128.

Option (b) feels simplest to me.

Thoughts?

Thanks, Jonathan