Hi Junio,

On Sun, 29 Sep 2019, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgad...@gmail.com>
> writes:
>
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index d1ba33745a..f21c781e68 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -695,7 +695,7 @@ test_failure_ () {
> >     say_color error "not ok $test_count - $1"
> >     shift
> >     printf '%s\n' "$*" | sed -e 's/^/#      /'
> > -   test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
> > +   test "$immediate" = "" || { finalize_junit_xml; GIT_EXIT_OK=t; exit 1; }
> >  }
>
> There are three places that do GIT_EXIT_OK=t in the test framework,
> and the above covers one of them.  The original in test_done is
> another, and that place is made to call the "finalize" thing (it
> used to have the same finalization code inlined).
>
> The remaining one appears in
>
>         error () {
>                 say_color error "error: $*"
>                 GIT_EXIT_OK=t
>                 exit 1
>         }
>
> I wonder if we should cover this case, too.  One caller of "error" I
> know is BUG that says "bug in the test script", which means that
> after successfully passing 30 tests, when the 31st test has 5 params
> to test_expect_success by mistake, without finailzation we will lose
> the result for the first 30.

Good point.

> And if we call "finalize" from the "error" helper, perhaps it makes
> even more sense to update the above manual exit in test_failure_ to
> do something like
>
>       if test -n "$immediate"
>       then
>               error "immediate exit after the first error"
>       fi
>
> to delegate the finalization.

This adds an additional message. I am not sure how many scripts/CI
integrations are there that rely on the current behavior, so I would
like to exclude this change from this here patch series: it is about
including a Visual Studio build in our Azure Pipeline, nothing more,
nothing less...

Ciao,
Dscho

>
> > @@ -1085,21 +1104,7 @@ test_done () {
> >     # removed, so the commands can access pidfiles and socket files.
> >     test_atexit_handler
> >
> > -   if test -n "$write_junit_xml" && test -n "$junit_xml_path"
> > -   then
> > -           test -n "$junit_have_testcase" || {
> > -                   junit_start=$(test-tool date getnanos)
> > -                   write_junit_xml_testcase "all tests skipped"
> > -           }
> > -
> > -           # adjust the overall time
> > -           junit_time=$(test-tool date getnanos $junit_suite_start)
> > -           sed "s/<testsuite [^>]*/& time=\"$junit_time\"/" \
> > -                   <"$junit_xml_path" >"$junit_xml_path.new"
> > -           mv "$junit_xml_path.new" "$junit_xml_path"
> > -
> > -           write_junit_xml "  </testsuite>" "</testsuites>"
> > -   fi
> > +   finalize_junit_xml
> >
> >     if test -z "$HARNESS_ACTIVE"
> >     then
>

Reply via email to