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

[Denton Liu <liu.denton@gmail.com>]

Hi Junio,

On Tue, Apr 21, 2020 at 01:44:08PM -0700, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > On Mon, Apr 20, 2020 at 01:09:49PM -0700, Junio C Hamano wrote:
> >> Eric Sunshine <sunshine@sunshineco.com> writes:
> >> 
> >> > 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?
> >
> > Thanks, this looks good to me too.
> 
> Thanks.  
> 
> So the 4-patch test-must-fail-fix series is now complete?  Whee.

Hannes suggested that we should drop the tip commit of this series[1]
and I tend to agree with him. Aside from that, though, the series is
ready to go.

(I could improve 3/8 as suggested here[2] but I'll throw it in the next
series instead since I'm trying to get into the habit of not adding in
unrelated patches.)

[1]: https://lore.kernel.org/git/6cfa2c447e1196d6c4325aff9fac52434d10fda8.1587372771.git.liu.denton@gmail.com/
[2]: https://lore.kernel.org/git/90faeb7a-1c6a-3fc6-6410-5e264c9340e8@kdbg.org/