Re: [PATCH 03/15] t0000: replace test_must_fail with ! for run_sub_test_lib_test()
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Dec 17, 2019, 20:23 UTC
- Message-ID
- <de3d194a-bb05-8049-673b-267f53a271d6@kdbg.org>
- In-Reply-To
- <fcfccebd7aaf120674691ba92a657802c2482d7e.1576583819.git.liu.denton@gmail.com>
Am 17.12.19 um 13:01 schrieb Denton Liu:
Show 22 quoted lines
> The test_must_fail function should only be used for git commands since > we should assume that external commands work sanely. We use > test_must_fail to test run_sub_test_lib_test() but that function does > not invoke any git commands internally. Replace these instances of > `test_must_fail` with `!`. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > t/t0000-basic.sh | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh > index 8a81a249d0..d60ad4b78b 100755 > --- a/t/t0000-basic.sh > +++ b/t/t0000-basic.sh > @@ -155,7 +155,7 @@ test_expect_success 'pretend we have a fully passing test suite' " > " > > test_expect_success 'pretend we have a partially passing test suite' " > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > partial-pass '2/3 tests passing' <<-\\EOF &&
It is a very uncommon situation (read: I doubt that it ever occurs) in our test suite that we expect a shell function to fail, but that we do *not* care at all which of its sub-commands actually failed. We actually do care which sub-command failed. Therefore, we have, e.g., the idiom "test_i18n_grep ! ...". And in fact, in the case of run_sub_test_lib_test we have the form run_sub_test_lib_test_err to check for error exit in the subordinate test. All of the cases you change here should use it.
Show 57 quoted lines
> test_expect_success 'passing test #1' 'true' > test_expect_success 'failing test #2' 'false' > @@ -219,7 +219,7 @@ test_expect_success 'pretend we have fixed one of two known breakages (run in su > " > > test_expect_success 'pretend we have a pass, fail, and known breakage' " > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > mixed-results1 'mixed results #1' <<-\\EOF && > test_expect_success 'passing test' 'true' > test_expect_success 'failing test' 'false' > @@ -238,7 +238,7 @@ test_expect_success 'pretend we have a pass, fail, and known breakage' " > " > > test_expect_success 'pretend we have a mix of all possible results' " > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > mixed-results2 'mixed results #2' <<-\\EOF && > test_expect_success 'passing test' 'true' > test_expect_success 'passing test' 'true' > @@ -274,7 +274,7 @@ test_expect_success 'pretend we have a mix of all possible results' " > " > > test_expect_success C_LOCALE_OUTPUT 'test --verbose' ' > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > t1234-verbose "test verbose" --verbose <<-\EOF && > test_expect_success "passing test" true > test_expect_success "test with output" "echo foo" > @@ -301,7 +301,7 @@ test_expect_success C_LOCALE_OUTPUT 'test --verbose' ' > ' > > test_expect_success 'test --verbose-only' ' > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > t2345-verbose-only-2 "test verbose-only=2" \ > --verbose-only=2 <<-\EOF && > test_expect_success "passing test" true > @@ -834,7 +834,7 @@ then > fi > > test_expect_success 'tests clean up even on failures' " > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > failing-cleanup 'Failing tests with cleanup commands' <<-\\EOF && > test_expect_success 'tests clean up even after a failure' ' > touch clean-after-failure && > @@ -863,7 +863,7 @@ test_expect_success 'tests clean up even on failures' " > " > > test_expect_success 'test_atexit is run' " > - test_must_fail run_sub_test_lib_test \ > + ! run_sub_test_lib_test \ > atexit-cleanup 'Run atexit commands' -i <<-\\EOF && > test_expect_success 'tests clean up even after a failure' ' > > ../../clean-atexit && >
-- Hannes