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

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.

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