Hi Eric,

On Tue, 4 Sep 2018, Eric Sunshine wrote:

On Mon, Sep 3, 2018 at 5:10 PM Johannes Schindelin via GitGitGadget
[off-list ref] wrote:

quoted

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

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
    }

Or we could implement `test_atexit` (similar to `test_when_finished`, but
to be executed at `test_done` time). I guess that's what the p4 and daemon
tests really needed to begin with (and probably also the apache2-using
tests).

quoted

+                       # 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).

Now that you mention it... I agree. This is getting long.

In the short run, I have two things to consider, though: I want to make
this work first, then think about introducing a layer of abstraction, and
I want to go on vacation tomorrow.

So I agree that this is something to be considered in the long run, i.e.
not right now ;-)

Thanks,
Dscho