Re: [PATCH] ci: avoid running the test suite _twice_
From: Jeff King <hidden>
Date: 2023-11-16 20:02:47
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Wed, Nov 15, 2023 at 01:28:49PM -0800, Josh Steadmon wrote:
The first part is easy, but I don't see a good way to get both shell tests and unit tests executing under the same `prove` process. For shell tests, we pass `--exec '$(TEST_SHELL_PATH_SQ)'` to prove, meaning that we use the specified shell as an interpreter for the test files. That will not work for unit test executables.
Yes, it's unfortunate that you can't set the "exec" flag per-script (especially because without --exec it will auto-detect the right thing, but then of course it won't use TEST_SHELL_PATH). But we can intercept and do it ourselves, like:
diff --git a/t/Makefile b/t/Makefile
index 225aaf78ed..0b7c028eea 100644
--- a/t/Makefile
+++ b/t/Makefile@@ -61,7 +61,7 @@ failed: test -z "$$failed" || $(MAKE) $$failed prove: pre-clean check-chainlint $(TEST_LINT) - @echo "*** prove ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) + @echo "*** prove ***"; TEST_SHELL_PATH='$(TEST_SHELL_PATH_SQ)' $(CHAINLINTSUPPRESS) $(PROVE) --exec ./run-test.sh $(GIT_PROVE_OPTS) $(T) $(UNIT_TESTS) :: $(GIT_TEST_OPTS) $(MAKE) clean-except-prove-cache $(T):
diff --git a/t/run-test.sh b/t/run-test.sh
new file mode 100755
index 0000000000..69944029c8
--- /dev/null
+++ b/t/run-test.sh@@ -0,0 +1,10 @@ +#!/bin/sh + +case "$1" in +*.sh) + exec ${TEST_SHELL_PATH:-/bin/sh} "$@" + ;; +*) + exec "$@" + ;; +esac
You can actually do this inside the prove script using their plugin interface, but the necessary bits are somewhat arcane.
We could bundle all the unit tests into a single shell script, but then we lose parallelization and add hoops to jump through to determine what breaks. Or we could autogenerate a corresponding shell script to run each individual unit test, but that seems gross. Of course, these are hypothetical concerns for now, since we only have a single unit test at the moment.
We can't just stick them all in a single script; there must be exactly
one "plan" line in the TAP output from a given source. I had imagined
just manually adding a thin wrapper for each ("t9970-unit-strbuf" or
something). But it would also be easy to autogenerate them while
compiling. (Although all of that is moot with the wrapper I showed
above).
There's also the issue that the shell test arguments we pass on from prove would be shared with the unit tests. That's fine for now, as t-strbuf doesn't accept any runtime arguments, but it's possible that either the framework or individual unit tests might grow to need arguments, and it might not be convenient to stay compatible with the shell tests.
Sharing the options between the two seems like a benefit to me. I'd think that "-v" and "-i" would be useful, at least. Options which don't apply (e.g., "--root") could be quietly ignored. -Peff