[PATCH v3 0/7] test-tool: add unit test suite runner

Please note: this series has once again been rebased onto the latest jk/unit-tests-buildfix.

For various reasons (see discussion at [1]) we would like an alternative to `prove` for running test suites (including the unit tests) on Windows.

[1] https://lore.kernel.org/git/pull.1613.git.1699894837844.gitgitgadget@gmail.com/

This series extends the existing `test-tool run-command testsuite` to
support running unit tests. In addition, it includes some small
cleanups:
* move t-basic out of the unit-tests directory
* don't hardcode the shell for running tests in `test-tool ... testsuite`
* don't hardcode a test name filter in `test-tool ... testsuite`
* add a test wrapper script to allow unit tests and the shell test suite
  to run in a single `prove` process

Changes in V3:
* Added new patch (#7) to use the new test-tool runner for unit tests in
  Windows CI.
* Restored the explicit sort call in t/Makefile, for backwards
  compatibility with older GNU Make versions.
* Rebased the series on the latest jk/unit-tests-buildfix branch.
* Fixed header include order in patch #1.
* Removed a paragraph in patch #1's commit message that is obsolete now
  that we're building the list of test files from the sources rather
  than by globbing.
* Added a note in patch #2 that setting a NULL suite.shell_path will be
  used in a later commit.
* Clarified up some sloppy wording in commit messages and comments in
  t/helper/test-run-command.c.

Changes in V2:
* Rebased the series on the latest jk/unit-tests-buildfix branch.
* Patch 1: move t-basic to a test-tool subcommand rather than a new
  executable under t/t0080/
* New patch 2: get the shell path from TEST_SHELL_PATH in
  `test-tool run-command testsuite`
* New patch 3: remove the hardcoded filename filter in
  `test-tool run-command testsuite`
* Patch 4 (previously 2): simplified now that we no longer need to add
  any command-line flags to support unit tests
* Patch 5 (previously 3): avoid trying to run cmake *.pdb files by using
  the unit test list built in the makefile in jk/unit-tests-buildfix.

Jeff King (1):
  t/Makefile: run unit tests alongside shell tests

Josh Steadmon (6):
  t0080: turn t-basic unit test into a helper
  test-tool run-command testsuite: get shell from env
  test-tool run-command testsuite: remove hardcoded filter
  test-tool run-command testsuite: support unit tests
  unit tests: add rule for running with test-tool
  ci: use test-tool as unit test runner on Windows

 Makefile                                      |  6 ++--
 ci/run-build-and-tests.sh                     |  2 --
 ci/run-test-slice.sh                          |  2 +-
 t/Makefile                                    | 14 ++++++++--
 .../t-basic.c => helper/test-example-tap.c}   |  5 ++--
 t/helper/test-run-command.c                   | 28 +++++++++++++++----
 t/helper/test-tool.c                          |  1 +
 t/helper/test-tool.h                          |  1 +
 t/run-test.sh                                 | 13 +++++++++
 t/t0080-unit-test-output.sh                   | 24 ++++++++--------
 10 files changed, 67 insertions(+), 29 deletions(-)
 rename t/{unit-tests/t-basic.c => helper/test-example-tap.c} (95%)
 create mode 100755 t/run-test.sh

Range-diff against v2:
1:  da756b4bfb ! 1:  6777451100 t0080: turn t-basic unit test into a helper
    @@ Commit message
         t0080-unit-test-output.sh, so let's move it to
         t/helper/test-example-tap.c and adjust Makefiles as necessary.
     
    -    This has the additional benefit that test harnesses seeking to run all
    -    unit tests can find them with a simple glob of "t/unit-tests/bin/t-*",
    -    with no exceptions needed. This will be important in a later patch where
    -    we add support for running the unit tests via a test-tool subcommand.
    -
     
      ## Makefile ##
    @@ Makefile: perf: all
      	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)
     
      ## t/Makefile ##
    -@@ t/Makefile: TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
    - CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
    +@@ t/Makefile: CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.tes
      CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
      UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
    --UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
    + UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
     -UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(UNIT_TEST_PROGRAMS)))
    -+UNIT_TESTS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
    ++UNIT_TESTS = $(sort $(UNIT_TEST_PROGRAMS))
      
      # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)
      # checks all tests in all scripts via a single invocation, so tell individual
    @@ t/Makefile: TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
      ## t/unit-tests/t-basic.c => t/helper/test-example-tap.c ##
     @@
     -#include "test-lib.h"
    -+#include "t/unit-tests/test-lib.h"
     +#include "test-tool.h"
    ++#include "t/unit-tests/test-lib.h"
      
      /*
       * The purpose of this "unit test" is to verify a few invariants of the unit
2:  c8448406d7 ! 2:  24f47f8fc7 test-tool run-command testsuite: get shell from env
    @@ Commit message
         Add a shell_path field in struct testsuite so that we can pass this to
         the task runner callback. If this is non-null, we'll use it as the
         argv[0] of the subprocess. Otherwise, we'll just execute the test
    -    program directly.
    +    program directly. We will use this feature in a later commit to enable
    +    running binary executable unit tests.
     
    -    When setting up the struct testsuite in testsuite(), use the value
    -    of TEST_SHELL_PATH if it's set, otherwise default to `sh`.
    +    However, for now when setting up the struct testsuite in testsuite(),
    +    use the value of TEST_SHELL_PATH if it's set, otherwise keep the
    +    original behavior by defaulting to `sh`.
     
         [1] https://lore.kernel.org/git/20240123005913.GB835964@coredump.intra.peff.net/
     
3:  e1b89ae93e = 3:  4a16a3ec24 test-tool run-command testsuite: remove hardcoded filter
4:  b5665386b5 ! 4:  abc9a7afe8 test-tool run-command testsuite: support unit tests
    @@ Commit message
         test-tool run-command testsuite: support unit tests
     
         Teach the testsuite runner in `test-tool run-command testsuite` how to
    -    run unit tests: if TEST_SHELL_PATH is not set, assume that we're running
    -    the programs directly from CWD, rather than defaulting to "sh" as an
    -    interpreter.
    +    run unit tests: if TEST_SHELL_PATH is not set, run the programs directly
    +    from CWD, rather than defaulting to "sh" as an interpreter.
     
         With this change, you can now use test-tool to run the unit tests:
         $ make
    @@ t/helper/test-run-command.c: static int testsuite(int argc, const char **argv)
      		max_jobs = online_cpus();
      
     +	/*
    -+	 * If we run without a shell, we have to provide the relative path to
    -+	 * the executables.
    ++	 * If we run without a shell, execute the programs directly from CWD.
     +	 */
      	suite.shell_path = getenv("TEST_SHELL_PATH");
      	if (!suite.shell_path)
5:  f2746703d5 ! 5:  a8bbff2c6b unit tests: add rule for running with test-tool
    @@ Makefile: $(UNIT_TEST_PROGS): $(UNIT_TEST_BIN)/%$X: $(UNIT_TEST_DIR)/%.o $(UNIT_
      	$(MAKE) -C t/ unit-tests
     
      ## t/Makefile ##
    -@@ t/Makefile: CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.tes
    - CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
    +@@ t/Makefile: CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
      UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
    - UNIT_TESTS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
    + UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
    + UNIT_TESTS = $(sort $(UNIT_TEST_PROGRAMS))
     +UNIT_TESTS_NO_DIR = $(notdir $(UNIT_TESTS))
      
      # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)
6:  cd7467a7bd ! 6:  cfcc4bd427 t/Makefile: run unit tests alongside shell tests
    @@ Commit message
         twice in CI, as discussed in [1].
     
         Additionally, this moves the unit tests into the main dev workflow, so
    -    that errors can be spotted more quickly.
    -
    -    NEEDS WORK: as discussed in previous commits in this series, there's a
    -    desire to avoid `prove` specifically and (IIUC) unnecessary
    -    fork()/exec()ing in general on Windows. This change adds an extra exec()
    -    for each shell and unit test execution, will that be a problem for
    -    Windows?
    +    that errors can be spotted more quickly. Accordingly, we remove the
    +    separate unit tests step for Linux CI. (We leave the Windows CI
    +    unit-test step as-is, because the sharding scheme there involves
    +    selecting specific test files rather than running `make test`.)
     
         [1] https://lore.kernel.org/git/pull.1613.git.1699894837844.gitgitgadget@gmail.com/
     
         Signed-off-by: Jeff King <peff@peff.net>
     
    + ## ci/run-build-and-tests.sh ##
    +@@ ci/run-build-and-tests.sh: if test -n "$run_tests"
    + then
    + 	group "Run tests" make test ||
    + 	handle_failed_tests
    +-	group "Run unit tests" \
    +-		make DEFAULT_UNIT_TEST_TARGET=unit-tests-prove unit-tests
    + fi
    + check_unignored_build_artifacts
    + 
    +
      ## t/Makefile ##
     @@ t/Makefile: failed:
      	test -z "$$failed" || $(MAKE) $$failed
-:  ---------- > 7:  cbf37e0ddc ci: use test-tool as unit test runner on Windows

base-commit: 4904a4d08cc085716df12ce713ae7ee3d5ecb75a