| Message ID | 20210228195414.21372-11-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | describe: dont abort too early when searching tags | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index c6cdabf53e..3dd68091bb 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -636,6 +636,7 @@ test_expect_failure () { > fi > fi > test_finish_ > + return 1 > } > > test_expect_success () { > @@ -656,6 +657,7 @@ test_expect_success () { > fi > fi > test_finish_ > + return 1 > } > > # test_external runs external test scripts that provide continuous Hmph. This does not catch if the outer expect_success does not catch a failure in the inner expect_success and signal a failure. When I asked if this kind of breakage is an easy mistake to catch by the test lint, I had something along this in mind: test_expect_success () { if test -n "$GIT_IN_TEST_EXPECT" then BUG caling "$GIT_IN_TEST_EXPECT" inside test_expect_success fi GIT_IN_TEST_EXPECT=test_expect_success ... do the 'eval the given test body' thing .. GIT_IN_TEST_EXPECT= } After all, the error is in the outer expect_success in that it called another one, so it feels more natural that the called inner expect_success to notice the situation and barf. Thanks.
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index c6cdabf53e..3dd68091bb 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -636,6 +636,7 @@ test_expect_failure () { fi fi test_finish_ + return 1 } test_expect_success () { @@ -656,6 +657,7 @@ test_expect_success () { fi fi test_finish_ + return 1 } # test_external runs external test scripts that provide continuous
An earlier commit fixed an issue in "check_describe" with "test_expect_success" being called within another "test_expect_success", causing the test to succeed even if it should fail. Let's try to guard against this in the test library by returning 1 from these two functions. This change would have caught the issue I've now fixed in the "check_describe" function. I could equivalently add this "return 1" to the "test_finish_" function itself, but I think doing it here is more readable. Because of this change any tests which ran under "set -e" needed to be refactored not to use "set -e". Luckily there were only two such tests, earlier commits did that refactoring. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/test-lib-functions.sh | 2 ++ 1 file changed, 2 insertions(+)