Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 4, 2018, 04:30 UTC
Message-ID: <CAPig+cRGHPfNqdgRr6H_D_siKCFZBDsUrcp0F+CWNCx884AJVg@mail.gmail.com>
In-Reply-To: <942bf423a461a3b44e2ff254c90907a539d7abbe.1536009027.git.gitgitgadget@gmail.com>

On Mon, Sep 3, 2018 at 5:10 PM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 21 quoted lines

> So let's do something different in VSTS: let's run all the tests with
> `--quiet` first, and only if a failure is encountered, try to trace the
> commands as they are executed. [...]
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> @@ -445,10 +452,37 @@ test_ok_ () {
>  test_failure_ () {
>         if test -n "$write_junit_xml"
>         then
> +               if test -z "$GIT_TEST_TEE_OUTPUT_FILE"
> +               then
> +                       case "$(type kill_p4d 2>/dev/null | head -n 1)" in
> +                       *function*) kill_p4d;;
> +                       esac
> +
> +                       case "$(type stop_git_daemon 2>/dev/null |
> +                               head -n 1)" in
> +                       *function*) stop_git_daemon;;
> +                       esac

In the long run, it might make more sense, and be more scalable, to have those scripts define a "prepare_for_rerun" variable or function which this code then runs generically rather than having special knowledge of those facilities.

I could imagine, for instance, test-lib.sh defining a no-op:

    test_failure_prepare_rerun () {}

and then each of those scripts overriding the function:

    # in lib-git-p4.sh
    test_failure_prepare_rerun () {
        kill_p4d
    }

    # in lib-git-daemon.sh
    test_failure_prepare_rerun () {
        stop_git_daemon
    }

Show 24 quoted lines

> +                       # re-run with --verbose-log
> +                       echo "# Re-running: $junit_rerun_options_sq" >&2
> +
> +                       cd "$TEST_DIRECTORY" &&
> +                       eval "${TEST_SHELL_PATH}" "$junit_rerun_options_sq" \
> +                               >/dev/null 2>&1
> +                       status=$?
> +
> +                       say_color "" "$(test 0 = $status ||
> +                               echo "not ")ok $test_count - (re-ran with trace)"
> +                       say "1..$test_count"
> +                       GIT_EXIT_OK=t
> +                       exit $status
> +               fi
> +
>                 junit_insert="<failure message=\"not ok $test_count -"
>                 junit_insert="$junit_insert $(xml_attr_encode "$1")\">"
>                 junit_insert="$junit_insert $(xml_attr_encode \
> -                       "$(printf '%s\n' "$@" | sed 1d)")"
> +                       "$(cat "$GIT_TEST_TEE_OUTPUT_FILE")")"
> +               >"$GIT_TEST_TEE_OUTPUT_FILE"
>                 junit_insert="$junit_insert</failure>"
>                 write_junit_xml_testcase "$1" "      $junit_insert"
>         fi

This junit-related stuff is getting pretty lengthy. I wonder if it would make sense to pull it out to its own function at some point (again, in the long run).