Re: [RFC PATCH] t: move metadata into TAP test description

From: Alban Gruin <alban.gruin@gmail.com>
Date: May 15, 2020, 15:38 UTC
Message-ID: <880ded78-21c6-9310-6c5e-422f6a63ad47@gmail.com>
In-Reply-To: <20200515150041.22873-1-carenas@gmail.com>

Le 15/05/2020 à 17:00, Carlo Marcelo Arenas Belón a écrit :

Show 48 quoted lines

> 662f9cf154 (tests: when run in Bash, annotate test failures with file
> name/line number, 2020-04-11) adds metadata to the TAP output to help
> identify the location of the failed test, but does it in a way that
> break the TAP format and therefore confuses prove.
> 
> Move the metadata to the description to workaround the issue and
> change the regex from 676eb0c1ce (ci: add a problem matcher for GitHub
> Actions, 2020-04-11) to match.
> 
> Reported-by: Alban Gruin <alban.gruin@gmail.com>
> ---
>  ci/git-problem-matcher.json | 10 +++++-----
>  t/test-lib.sh               |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/ci/git-problem-matcher.json b/ci/git-problem-matcher.json
> index 506dfbd97f..e10e88bba1 100644
> --- a/ci/git-problem-matcher.json
> +++ b/ci/git-problem-matcher.json
> @@ -4,11 +4,11 @@
>              "owner": "git-test-suite",
>              "pattern": [
>                  {
> -                    "regexp": "^([^ :]+\\.sh):(\\d+): (error|warning|info):\\s+(.*)$",
> -                    "file": 1,
> -                    "line": 2,
> -                    "severity": 3,
> -                    "message": 4
> +                    "regexp": "^(.*)(error|warning|info):\\([^ :]+\\.sh):(\\d+)\\)$",
> +                    "file": 3,
> +                    "line": 4,
> +                    "severity": 2,
> +                    "message": 1
>                  }
>              ]
>          }
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index baf94546da..d5f59ab3bf 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -682,8 +682,8 @@ file_lineno () {
>  		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;;
> +			0,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:$LINENO)"; return;;
> +			*,t[0-9]*.sh) echo "(${1+$1:}t/${BASH_SOURCE[$i]}:${BASH_LINENO[$(($i-1))]})"; return;;

Could you add a space after the "error:"/"warn:"/"info:"?

Show 9 quoted lines

>  			esac
>  		done
>  	'
> @@ -734,7 +734,7 @@ test_failure_ () {
>  		write_junit_xml_testcase "$1" "      $junit_insert"
>  	fi
>  	test_failure=$(($test_failure + 1))
> -	say_color error "$(file_lineno error)not ok $test_count - $1"
> +	say_color error "not ok $test_count - $1$(file_lineno error)"

Could you add a space before `$(file_lineno error)'?

>  	shift
>  	printf '%s\n' "$*" | sed -e 's/^/#	/'
>  	test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
>

Thinking about it, I think it would make sense to put this kind of information in a diagnostic line, as we already do for the code of a failed test.

Instead of this:

Show 6 quoted lines

> not ok 1 - plain(error:t/./t0001-init.sh:33)
> #
> #               false &&
> #               git init plain &&
> #               check_config plain/.git false unset
> #

Something like this:

Show 7 quoted lines

> not ok 1 - plain
> # (error:t/./t0001-init.sh:33)
> #
> #               false &&
> #               git init plain &&
> #               check_config plain/.git false unset
> #

Cheers, Alban