On Mon, Apr 20, 2020 at 01:09:49PM -0700, Junio C Hamano wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > > On Mon, Apr 20, 2020 at 4:55 AM Denton Liu <liu.denton@xxxxxxxxx> 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@xxxxxxxxx> > >> --- > >> 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.
