Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number

From: Alban Gruin <alban.gruin@gmail.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>, git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v5 10/12] tests: when run in Bash, annotate test failures with file name/line number
Date: Fri, 15 May 2020 15:16:38 +0200	[thread overview]
Message-ID: <5b373748-d986-1aec-670f-7ac1502e7052@gmail.com> (raw)
In-Reply-To: <ce0038598704e03becc65731a71abf596bdfbe98.1586538752.git.congdanhqx@gmail.com>

Hi,

Le 10/04/2020 à 19:18, Đoàn Trần Công Danh a écrit :
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> When a test fails, it is nice to see where the corresponding code lives
> in the worktree. Sadly, it seems that only Bash allows us to infer this
> information. Let's do it when we detect that we're running in a Bash.
> 
> This will come in handy in the next commit, where we teach the GitHub
> Actions workflow to annotate failed test runs with this information.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  t/test-lib.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0ea1e5a05e..40a00983f7 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -657,6 +657,18 @@ die () {
>  	fi
>  }
>  
> +file_lineno () {
> +	test -z "$GIT_TEST_FRAMEWORK_SELFTEST" && test -n "$BASH" || return 0
> +	local i
> +	for i in ${!BASH_SOURCE[*]}
> +	do
> +		case $i,"${BASH_SOURCE[$i]##*/}" in
> +		0,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:$LINENO: ${1+$1: }"; return;;
> +		*,t[0-9]*.sh) echo "t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]}: ${1+$1: }"; return;;
> +		esac
> +	done
> +}
> +
>  GIT_EXIT_OK=
>  trap 'die' EXIT
>  # Disable '-x' tracing, because with some shells, notably dash, it
> @@ -702,7 +714,7 @@ test_failure_ () {
>  		write_junit_xml_testcase "$1" "      $junit_insert"
>  	fi
>  	test_failure=$(($test_failure + 1))
> -	say_color error "not ok $test_count - $1"
> +	say_color error "$(file_lineno error)not ok $test_count - $1"
>  	shift
>  	printf '%s\n' "$*" | sed -e 's/^/#	/'
>  	test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
> 

This violates the TAP specification, it seems.  Without this patch, when
a test fails, `prove' shows clearly which test(s) broke, and how many
failed:

> Test Summary Report
> -------------------
> t0001-init.sh (Wstat: 256 Tests: 45 Failed: 1)
>   Failed test:  2
>   Non-zero exit status: 1

With this patch, it shows this:

> Test Summary Report
> -------------------
> t0001-init.sh (Wstat: 256 Tests: 44 Failed: 0)
>   Non-zero exit status: 1
>   Parse errors: Tests out of sequence.  Found (3) but expected (2)
>                 Tests out of sequence.  Found (4) but expected (3)
>                 Tests out of sequence.  Found (5) but expected (4)
>                 Tests out of sequence.  Found (6) but expected (5)
>                 Tests out of sequence.  Found (7) but expected (6)
> Displayed the first 5 of 44 TAP syntax errors.
> Re-run prove with the -p option to see them all.

Not nice to see clearly which tests failed: we do not see all failed
tests (what if a test failed after the 7th test?), and the counter
displays 0.

This happens on my machine, as well as on github.

Cheers,
Alban