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

From: Randall S. Becker <rsbecker@nexbridge.com>
Date: Feb 28, 2018, 18:21 UTC
Message-ID: <005901d3b0c0$f5acd370$e1067a50$@nexbridge.com>
In-Reply-To: <20180228174402.GC251290@aiede.svl.corp.google.com>

On February 28, 2018 12:44 PM, Jonathan Nieder wrote:

Show 11 quoted lines

> 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

Show 15 quoted lines

> 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 #