On 4 September 2018 at 12:09, Johannes Schindelin
[off-list ref] wrote:

Hi Eric,

On Tue, 4 Sep 2018, Eric Sunshine wrote:

quoted

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. [...]

Is this re-running just an individual test on its own or all the tests
within a single file?

The latter shouldn't need this at all. And the former, I'm not sure
will actually work - most of the tests assume some particular p4
state. But perhaps I'm missing something?

I also think it does look kind of ugly. And if there's one thing I've
learned, it's that the ugly hack you write today with the words "we'll
tidy this up later" goes on to live with you forever!

quoted

quoted

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

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