Denton Liu <liu.denton@xxxxxxxxx> writes: > 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. Thanks. So the 4-patch test-must-fail-fix series is now complete? Whee.
