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

["Randall S. Becker" <rsbecker@nexbridge.com>]

On February 28, 2018 12:44 PM, Jonathan Nieder wrote:
> Randall S. Becker wrote:
> 
> > The problem is actually in git code in its test suite that uses perl
> > inline, not in my test code itself. The difficulty I'm having is
> > placing this appropriate so that the signal handler gets used
> > throughout the test suite including in the perl -e invocations. This
> > is more a lack of my own understanding of plumbing of git test
> > framework rather than of using or coding perl.
> 
> Can you elaborate with an example?  My understanding was that
> test_must_fail is only for running git.  If a test is running perl and
wants to
> check its exit code, the test is supposed to use !, not test_must_fail.
> 
> t/README backs me up:
> 
>  - use '! git cmd' when you want to make sure the git command exits
>    with failure in a controlled way by calling "die()".  Instead,
>    use 'test_must_fail git cmd'.  This will signal a failure if git
>    dies in an unexpected way (e.g. segfault).
> 
>    On the other hand, don't use test_must_fail for running regular
>    platform commands; just use '! cmd'.  We are not in the business
>    of verifying that the world given to us sanely works.
> 
> So I don't consider the initial issue you raised a test issue at all!
> It's a bug in the git commands, and a fix for it should not be specific to
the
> test suite.
> 
> And now it sounds like there is a second issue: the test suite is
overusing
> test_must_fail in some context and that needs to be fixed as well.

Have a look at a recent t1404 as a sample. Line 615 is the one causing the
platform grief, because it triggers a 'die'. However, the particular test
case #54, had no difference on platform with test_must_fail or !, which has
the same underlying EBADF completion after digging and digging.

not ok 52 - delete fails cleanly if packed-refs file is locked
#
#               prefix=refs/locked-packed-refs &&
#               # Set up a reference with differing loose and packed
versions:
#               git update-ref $prefix/foo $C &&
#               git pack-refs --all &&
#               git update-ref $prefix/foo $D &&
#               git for-each-ref $prefix >unchanged &&
#               # Now try to delete it while the `packed-refs` lock is held:
#               : >.git/packed-refs.lock &&
#               test_when_finished "rm -f .git/packed-refs.lock" &&
#               ! git update-ref -d $prefix/foo >out 2>err &&
#               git for-each-ref $prefix >actual &&
#               test_i18ngrep "Unable to create $Q.*packed-refs.lock$Q: File
exists" err &&
#               test_cmp unchanged actual
#