Re: [PATCH] test-lib: have --immediate emit valid TAP on failure

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 24, 2022, 13:38 UTC
Message-ID: <nycvar.QRO.7.76.6.2203241434360.388@tvgsbejvaqbjf.bet>
In-Reply-To: <patch-1.1-47b236139e6-20220323T204410Z-avarab@gmail.com>

Hi Ævar,

On Wed, 23 Mar 2022, Ævar Arnfjörð Bjarmason wrote:

Show 31 quoted lines

> Change the "--immediate" option so that it emits valid TAP on
> failure. Before this it would omit the required plan at the end,
> e.g. under SANITIZE=leak we'd show a "No plan found in TAP output"
> error from "prove":
>
>     $ prove t0006-date.sh ::  --immediate
>     t0006-date.sh .. Dubious, test returned 1 (wstat 256, 0x100)
>     Failed 1/22 subtests
>
>     Test Summary Report
>     -------------------
>     t0006-date.sh (Wstat: 256 Tests: 22 Failed: 1)
>       Failed test:  22
>       Non-zero exit status: 1
>       Parse errors: No plan found in TAP output
>     Files=1, Tests=22,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.18 cusr  0.06 csys =  0.27 CPU)
>     Result: FAIL
>
> Now we'll emit output that doesn't result in TAP parsing failures:
>
>     $ prove t0006-date.sh ::  --immediate
>     t0006-date.sh .. Dubious, test returned 1 (wstat 256, 0x100)
>     Failed 1/22 subtests
>
>     Test Summary Report
>     -------------------
>     t0006-date.sh (Wstat: 256 Tests: 22 Failed: 1)
>       Failed test:  22
>       Non-zero exit status: 1
>     Files=1, Tests=22,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.19 cusr  0.05 csys =  0.26 CPU)
>     Result: FAIL

The commit message is strong on the what, very strong in giving verbose output that might or might not clarify the intention, and a little weak in the why and the greater context.

In the broader story arc, I wonder why we're even bothering with TAP anymore because it seems that the world has moved on to more powerful file formats to represent test output, such as JUnit XML, that can be parsed, rendered and analyzed by powerful special-purpose applications.

Apart from taking reviewer time away from such more powerful avenues to make sense of our failing tests, the diff does not really hurt, so I have no objections against integrating this patch.

Ciao, Johannes

Show 45 quoted lines

> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 9dcbf518a78..17a268ccd1b 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -101,6 +101,19 @@ test_expect_success 'subtest: 2/3 tests passing' '
>  	EOF
>  '
>
> +test_expect_success 'subtest: --immediate' '
> +	run_sub_test_lib_test_err partial-pass \
> +		--immediate &&
> +	check_sub_test_lib_test_err partial-pass \
> +		<<-\EOF_OUT 3<<-EOF_ERR
> +	> ok 1 - passing test #1
> +	> not ok 2 - failing test #2
> +	> #	false
> +	> 1..2
> +	EOF_OUT
> +	EOF_ERR
> +'
> +
>  test_expect_success 'subtest: a failing TODO test' '
>  	write_and_run_sub_test_lib_test failing-todo <<-\EOF &&
>  	test_expect_success "passing test" "true"
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 515b1af7ed4..4373f7d70b5 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -806,7 +806,11 @@ test_failure_ () {
>  	say_color error "not ok $test_count - $1"
>  	shift
>  	printf '%s\n' "$*" | sed -e 's/^/#	/'
> -	test "$immediate" = "" || _error_exit
> +	if test -n "$immediate"
> +	then
> +		say_color error "1..$test_count"
> +		_error_exit
> +	fi
>  }
>
>  test_known_broken_ok_ () {
> --
> 2.35.1.1452.ga7cfc89151f
>
>