Re: [PATCH] tests: fix incorrect --write-junit-xml code

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 14, 2022, 16:14 UTC
Message-ID: <xmqq35f38yeb.fsf@gitster.g>
In-Reply-To: <pull.1288.git.1657789234416.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

>     Unfortunately, I noticed this regression no earlier than when I needed
>     to validate Git for Windows v2.37.1. Since v2.37.1 was an embargoed
>     release, I could not use GitHub Actions for the CI testing, so I had to
>     reinstate Git's Azure Pipeline.

I wonder if it would make your life easier if the same GitHub Actions CI stuff were available for the Cabal repository we use for embargoed work, by allowing you to use the same validation for usual releases and the enbargoed ones?

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 37 quoted lines

>  t/test-lib-junit.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh
> index c959183c7e2..79c31c788b9 100644
> --- a/t/test-lib-junit.sh
> +++ b/t/test-lib-junit.sh
> @@ -46,7 +46,7 @@ finalize_test_case_output () {
>  	shift
>  	case "$test_case_result" in
>  	ok)
> -		set "$*"
> +		set -- "$*"
>  		;;
>  	failure)
>  		junit_insert="<failure message=\"not ok $test_count -"
> @@ -65,17 +65,17 @@ finalize_test_case_output () {
>  			junit_insert="$junit_insert<system-err>$(xml_attr_encode \
>  				"$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")</system-err>"
>  		fi
> -		set "$1" "      $junit_insert"
> +		set -- "$1" "      $junit_insert"
>  		;;
>  	fixed)
> -		set "$* (breakage fixed)"
> +		set -- "$* (breakage fixed)"
>  		;;
>  	broken)
> -		set "$* (known breakage)"
> +		set -- "$* (known breakage)"
>  		;;
>  	skip)
>  		message="$(xml_attr_encode --no-lf "$skipped_reason")"
> -		set "$1" "      <skipped message=\"$message\" />"
> +		set -- "$1" "      <skipped message=\"$message\" />"
>  		;;
>  	esac

OK. Ancient shells did not understand "--" and it was idiomatic to say "set x ...; shift", but we already do assume "set --" is usable everywhere we care about in many of our scripts and tests.

Looks good to me.

Thanks.  Will queue.