Hi Junio, On Tue, Apr 21, 2020 at 01:44:08PM -0700, Junio C Hamano wrote: > 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. 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@xxxxxxxxx/ [2]: https://lore.kernel.org/git/90faeb7a-1c6a-3fc6-6410-5e264c9340e8@xxxxxxxx/
