Re: [PATCH v2] t/Makefile: add a rule to re-run previously-failed tests

On Mon, Aug 29, 2016 at 3:46 PM, Johannes Schindelin
[off-list ref] wrote:

While developing patch series, it is a good practice to run the test
suite from time to time, just to make sure that obvious bugs are caught
early.  With complex patch series, it is common to run `make -j15 -k
test`, i.e.  run the tests in parallel and *not* stop at the first
failing test but continue. This has the advantage of identifying
possibly multiple problems in one big test run.

It is particularly important to reduce the turn-around time thusly on
Windows, where the test suite spends 45 minutes on the computer on which
this patch was developed.

It is the most convenient way to determine which tests failed after
running the entire test suite, in parallel, to look for left-over "trash
directory.t*" subdirectories in the t/ subdirectory. However, as was
pointed out by Jeff King, those directories might live outside t/ when
overridden using the --root=<directory> option, to which the Makefile
has no access. The next best method is to grep explicitly for failed
tests in the test-results/ directory, which the Makefile *can* access.

This patch automates the process of determinig which tests failed
previously and re-running them.

Note that we need to be careful to inspect only the *newest* entries in
test-results/: this directory contains files of the form
tNNNN-<name>-<pid>.counts and is only removed wholesale when running the
*entire* test suite, not when running individual tests. We ensure that
with a little sed magic on `ls -t`'s output that simply skips lines
when the file name was seen earlier.

Signed-off-by: Johannes Schindelin <redacted>
---

        The patch is unfortunately no longer as trivial as before, but it
        now also works with --root=..., i.e. when the user overrode the
        location of the trash directories.

Published-As: https://github.com/dscho/git/releases/tag/failing-tests-v2
Fetch-It-Via: git fetch https://github.com/dscho/git failing-tests-v2
Interdiff vs v1:

 diff --git a/t/Makefile b/t/Makefile
 index c402a9ec..8aa6a72 100644
 --- a/t/Makefile
 +++ b/t/Makefile
 @@ -35,7 +35,12 @@ all: $(DEFAULT_TEST_TARGET)
  test: pre-clean $(TEST_LINT)
        $(MAKE) aggregate-results-and-cleanup

 -failed: $(patsubst trash,,$(patsubst directory.%,%.sh,$(wildcard trash\ directory.t[0-9]*)))
 +failed:
 +      @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
 +              grep -l '^failed [1-9]' $$(ls -t *.counts | \
 +                      sed 'G;h;/^\(t[^.]*\)-[0-9]*\..*\n\1-[0-9]*\./d;P;d') | \
 +              sed -n 's/-[0-9]*\.counts$$/.sh/p') && \
 +      test -z "$$failed" || $(MAKE) $$failed

  prove: pre-clean $(TEST_LINT)
        @echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)

I don't at all mind this solution to the problem, if it works for that's cool.

But FWIW something you may have missed is that you can just use
prove(1) for this, which is why I initially patched git.git to support
TAP, so I didn't have to implement stuff like this.

I.e.:

    $ prove --state=save t[0-9]*.sh
    $ prove --state=failed,save t[0-9]*.sh

Does exactly what you're trying to do here with existing tools we support.

I.e. your new target could just be implemented in terms of calling prove.

Check out its man page, it may have other stuff you want to use, e.g.
you can run the slowest tests first etc.