Hi Junio,
On Sun, 29 Sep 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
> 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
>
