Re: [PATCH 6/8] t9164: don't use `test_must_fail test_cmp`

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 20, 2020, 20:09 UTC
Message-ID: <xmqqr1whrkaa.fsf@gitster.c.googlers.com>
In-Reply-To: <CAPig+cQ6XS=ZDhAKGuDiGM4zcoxUhnghMY250qYLjuT8YZaoMg@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 14 quoted lines

> On Mon, Apr 20, 2020 at 4:55 AM Denton Liu <liu.denton@gmail.com> wrote:
>> The test_must_fail function should only be used for git commands since
>> we assume that external commands work sanely. Since test_cmp() just
>> wraps an external command, replace `test_must_fail test_cmp` with
>> `! test_cmp`.
>>
>> Signed-off-by: Denton Liu <liu.denton@gmail.com>
>> ---
>> diff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh
>> @@ -103,7 +103,7 @@ test_expect_success 'check if pre-commit hook fails' '
>> -               test_must_fail svn_cmd commit -m "this commit should fail" &&
>> +               ! svn_cmd commit -m "this commit should fail" &&
>
> Hmm, this doesn't look like 'test_cmp' mentioned in the commit message.

Yeah, the other hunk is about test_cmp and this hunk is about svn_cmd. The stated rationale applies to both wrappers, I think.

    Subject: [PATCH 6/8] t9164: use test_must_fail only on git

    The `test_must_fail` function should only be used for git commands;
    we are not in the business of catching segmentation fault by external
    commands.  Shell helper functions test_cmp and svn_cmd used in this
    script are wrappers around external commands, so just use `! cmd`
    instead of `test_must_fail cmd`

perhaps, without any change to the code?