Re: [PATCH v9 2/8] t2400: print captured git output when finished

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 18, 2023, 16:34 UTC
Message-ID: <xmqqsfcxdu9k.fsf@gitster.g>
In-Reply-To: <olztmib77r35mx33a655obqpxui6coj74hfxoxfvcudnkpbqns@ixerneqaai45>

Jacob Abel <jacobabel@nullpo.dev> writes:

Show 6 quoted lines

>> So "sh t2400-*.sh -x -i" already shows "cat actual" output.  Try
>> the attached patch on top of this one and running it would show
>> the above message shown by test_must_be_empty and the contents of
>> the file 'actual'.
>
> That should not be the case.

Here is how it fails and test_must_be_empty complains that the "actual" file is not empty, if you run the script with "-x -i" after applying the patch in the message you are responding to on top of this step.

    expecting success of 2400.37 'add --quiet':
            test_when_finished "git worktree remove -f -f another-worktree" &&
            test_when_finished cat actual >&2 &&
            git worktree add --quiet another-worktree main 2>actual &&
    echo foo >>actual &&
            test_must_be_empty actual

    ++ test_when_finished 'git worktree remove -f -f another-worktree'
    ++ test 0 = 0
    ++ test_cleanup='{ git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ test_when_finished cat actual
    ++ test 0 = 0
    ++ test_cleanup='{ cat actual
                    } && (exit "$eval_ret"); eval_ret=$?; { git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ git worktree add --quiet another-worktree main
    ++ echo foo
    ++ test_must_be_empty actual
    ++ test 1 -ne 1
    ++ test_path_is_file actual
    ++ test 1 -ne 1
    ++ test -f actual
    ++ test -s actual
    ++ echo ''\''actual'\'' is not empty, it contains:'
    'actual' is not empty, it contains:
    ++ cat actual
    foo
    ++ return 1
    error: last command exited with $?=1
    not ok 37 - add --quiet

Observe what test_must_be_empty does in the last part of the transcript above. If you run it without "-i", then the trace will show "cat actual" twice (one from test_must_be_empty above, then a redundant one from the test_when_finished).

Another reason why we shouldn't add this to test_when_finished is because it would not help those who run the tests with "-i" option. The test_cleanup handlers are meant to be "clean-up" routines, and they are not run when the user uses "-i", intending to go into the test directory after seeing the test fail and inspect what is left there.