Re: [PATCH] t7450: make test "set -e" clean

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Mar 24, 2026 at 2:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>> In order to catch mistakes like misspelling "test_expect_success",
>> we would like to eventually be able to run our test suite with the
>> "-e" option on.
>>
>> Often we write "A && test_expect_success ..." and want it to mean
>> "If and only if A holds true, this needs to be tested", but under
>> "set -e", this will cause failure when A does not hold true.  We
>> need to write "!A || test_expect_success ..." if we want to run the
>> test conditionally.
>>
>> Or write it properly with if/then/fi, perhaps like:
>>
>>         if ! A
>>         then
>>                 test_expect_success ...
>>         fi
>>
>> Make sure we do not fail unnecessarily under "set -e".
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git i/t/t7450-bad-git-dotfiles.sh w/t/t7450-bad-git-dotfiles.sh
>> @@ -220,7 +220,7 @@ check_dotx_symlink () {
>> -       test -n "$refuse_index" &&
>> +       test -z "$refuse_index" ||
>>         test_expect_success "refuse to load symlinked $name into index ($type)" '
>>                 test_must_fail \
>>                         git -C $dir \
>
> I suppose this is the absolute minimum change to make this work, but
> typically we would handle this sort of case by defining a PREREQ,
> wouldn't we? Using a PREREQ would also set a better example for those
> new to the codebase.

In some situations, maybe, but I do not think this one is a good fit
for a prerequisite, whose typical pattern is "let's see what we have
in the executing platform environment just once, and act accordingly".

This is a "the outside helper function is repeatedly called, and the
caller may or may not call it with an option, depending on which
this extra test may or may not make sense to run, so run this one
conditionally".