Git fork

test-lib: fail on unexpectedly passing tests

When tests are executed via `test_expect_failure` we rather obviously
expect the test itself to fail. If it unexpectedly does not fail then we
count the test as a "fixed" test and announce that a known breakage has
vanished:

ok 1 - setup
ok 2 - create refs/heads/main # TODO known breakage vanished
ok 3 - create refs/heads/main with oldvalue verification
...
ok 299 - update-ref should also create reflog for HEAD
# 1 known breakage(s) vanished; please update test(s)
# passed all remaining 298 test(s)
1..299

While we announce that tests should be updated, the overall test suite
still passes. This makes it quite hard to detect when a test that has
previously failed succeeds now as the developer needs to pay close
attention to the exact output. Even more importantly, tests that only
succeed on _some_ systems are even easier to miss now, as one would have
to explicitly take a look at respective CI jobs to notice that those do
pass now.

Furthermore, we are about to introduce support for parsing TAP output in
Meson. In contrast to prove(1), which treats unexpected passes as a
successful test run, Meson treats those as failure. Neither of these
tools is wrong in doing so. Quoting the TAP specification [1]:

Should a todo test point begin succeeding, the harness may report it
in some way that indicates that whatever was supposed to be done has
been, and it should be promoted to a normal Test Point.

So it is essentially implementation-defined how exactly the unexpected
pass is reported, and whether it should cause the overall test suite to
fail or not. It is unarguably a bad thing for us though if these tools
interpret these differently, as it would mean that test results now
depend on whether the developer uses prove(1) or Meson.

Unify the behaviour by causing a test suite to fail when there are any
unexpected passes. As prove(1) does not consider an unexpected pass to
be an error this leads to somewhat funky output:

t1400-update-ref.sh ................................ Dubious, test returned 1 (wstat 256, 0x100)
All 299 subtests passed
(1 TODO test unexpectedly succeeded)

...

Test Summary Report
-------------------
t1400-update-ref.sh (Wstat: 256 (exited 1) Tests: 299 Failed: 0)
TODO passed: 2
Non-zero exit status: 1

But as we directly announce that the root cause is an unexpected TODO
that has succeeded it's not all that bad.

[1]: https://testanything.org/tap-version-14-specification.html

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Patrick Steinhardt and committed by
Junio C Hamano
5e0752b0 d3d8c601

+10 -3
+2 -2
t/t0000-basic.sh
··· 130 130 ' 131 131 132 132 test_expect_success 'subtest: a passing TODO test' ' 133 - write_and_run_sub_test_lib_test passing-todo <<-\EOF && 133 + write_and_run_sub_test_lib_test_err passing-todo <<-\EOF && 134 134 test_expect_failure "pretend we have fixed a known breakage" "true" 135 135 test_done 136 136 EOF ··· 142 142 ' 143 143 144 144 test_expect_success 'subtest: 2 TODO tests, one passin' ' 145 - write_and_run_sub_test_lib_test partially-passing-todos <<-\EOF && 145 + write_and_run_sub_test_lib_test_err partially-passing-todos <<-\EOF && 146 146 test_expect_failure "pretend we have a known breakage" "false" 147 147 test_expect_success "pretend we have a passing test" "true" 148 148 test_expect_failure "pretend we have fixed another known breakage" "true"
+8 -1
t/test-lib.sh
··· 1272 1272 1273 1273 check_test_results_san_file_ "$test_failure" 1274 1274 1275 - if test -z "$skip_all" && test -n "$invert_exit_code" 1275 + if test "$test_fixed" != 0 1276 + then 1277 + if test -z "$invert_exit_code" 1278 + then 1279 + GIT_EXIT_OK=t 1280 + exit 1 1281 + fi 1282 + elif test -z "$skip_all" && test -n "$invert_exit_code" 1276 1283 then 1277 1284 say_color warn "# faking up non-zero exit with --invert-exit-code" 1278 1285 GIT_EXIT_OK=t