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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Thu, 14 Jul 2022, Junio C Hamano wrote:

> "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

It would make my life easier if Chesterton's Fences were left alone ;-)

> 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?

GitHub Actions are available in the Cabal repository. The problem is that
due to the private nature of said repository, the number of included build
minutes is limited.

I did manage to get GitHub to sponsor the Git project specifically to
increase said build minutes. But that only scratches the problem at the
surface.

I still think that we need to slow the heck down with refactoring for
refactoring's sake because it's not only the CI builds that are affected.
I pay a lot of time to accommodate for those refactorings, and so do
others, and the benefit of most of those refactorings escapes me. My best
guess is that they adapt Git's source code to individual tastes, which is
of course a losing game because too many individuals with too many
different tastes are involved in the Git project. Unless we start valuing
particular individuals' tastes over others', which I believe we should not
do.

You asked me in private to provide more reviews for those refactorings so
that they see some push-back, but I lack the bandwidth for that.

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> >  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.

Thank you,
Dscho