Re: [PATCH] tests: fix incorrect --write-junit-xml code
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 8, 2022, 13:40 UTC
- Message-ID
- <s4s2qr56-2948-p025-rrq7-qq56p9oo844r@tzk.qr>
- In-Reply-To
- <xmqq35f38yeb.fsf@gitster.g>
Hi Junio,
On Thu, 14 Jul 2022, Junio C Hamano wrote:
Show 9 quoted lines
> "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.
Show 48 quoted lines
> "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