This gets rid of this fixed cost when running "make T=<name>":
$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C t check-chainlint' 'make -C t check-chainlint'
Benchmark 1: make -C t check-chainlint' in 'HEAD~1
Time (mean ± σ): 111.3 ms ± 0.5 ms [User: 116.9 ms, System: 26.2 ms]
Range (min … max): 110.5 ms … 112.5 ms 26 runs
Benchmark 2: make -C t check-chainlint' in 'HEAD~0
Time (mean ± σ): 12.5 ms ± 0.2 ms [User: 11.5 ms, System: 1.0 ms]
Range (min … max): 12.1 ms … 13.2 ms 223 runs
Summary
'make -C t check-chainlint' in 'HEAD~0' ran
8.92 ± 0.16 times faster than 'make -C t check-chainlint' in 'HEAD~1'
---
On Mon, Dec 13 2021, Eric Sunshine wrote:
> Rather than running `chainlint` and `diff` once per self-test -- which
> may become expensive as more tests are added -- instead run `chainlint`
> a single time over all tests bodies collectively and compare the result
> to the collective "expected" output.
I think that "optimizing" things like this is an anti-pattern. I.e. we
have N chainlint test files, and N potential outputs from that (ok or
not, and with/without error). If one of the chainlint tests changes
we'd like to re-run it, if not we can re-use an earlier run.
This is something make's dependency logic is perfectly suited for, and
will be faster than any optimization of turning a for-loop into a
"sed" command we run every time, since we'll only need to "stat" a few
things to see that there's nothing to do.
I've had the below as part of my local build for a while. It
conflicts/would be improved by my in-flight ab/make-dependency, which
you also ran into conflicts with.
This also has the advantage that you'll see what specific test failed
from the Makefile output. Aside from the ".build" rule needing to be
fixed up to use the "mkdir" trick in ab/make-dependency it should also
use $(QUIET_GEN) or whatever.
I'm not quite happy with the below, the wildcard/patsubst is a bit
messy, and it doesn't properly work around "make test" and how it runs
"clean" after its run, but you'll find that "make chainlint" will
properly cache things with this.
t/.gitignore | 1 +
t/Makefile | 29 +++++++++++++++--------------
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/t/.gitignore b/t/.gitignore
index 91cf5772fe5..f142d6d42fd 100644
--- a/t/.gitignore
+++ b/t/.gitignore
@@ -1,3 +1,4 @@
+/.build/
/trash directory*
/test-results
/.prove
diff --git a/t/Makefile b/t/Makefile
index 882d26eee30..9bac0e683c9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -18,10 +18,8 @@ TEST_LINT ?= test-lint
ifdef TEST_OUTPUT_DIRECTORY
TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
-CHAINLINTTMP = $(TEST_OUTPUT_DIRECTORY)/chainlinttmp
else
TEST_RESULTS_DIRECTORY = test-results
-CHAINLINTTMP = chainlinttmp
endif
# Shell quote;
@@ -29,13 +27,12 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
-CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
-CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
+CHAINLINTTESTS = $(wildcard chainlint/*.test)
CHAINLINT = sed -f chainlint.sed
all: $(DEFAULT_TEST_TARGET)
@@ -67,16 +64,20 @@ clean: clean-except-prove-cache
$(RM) .prove
clean-chainlint:
- $(RM) -r '$(CHAINLINTTMP_SQ)'
-
-check-chainlint:
- @mkdir -p '$(CHAINLINTTMP_SQ)' && \
- err=0 && \
- for i in $(CHAINLINTTESTS); do \
- $(CHAINLINT) <chainlint/$$i.test | \
- sed -e '/^# LINT: /d' >'$(CHAINLINTTMP_SQ)'/$$i.actual && \
- diff -u chainlint/$$i.expect '$(CHAINLINTTMP_SQ)'/$$i.actual || err=1; \
- done && exit $$err
+ $(RM) -r .build/chainlint
+
+BUILT_CHAINLINTTESTS = $(patsubst %,.build/%.actual,$(CHAINLINTTESTS))
+
+.build/chainlint:
+ mkdir -p $@
+
+$(BUILT_CHAINLINTTESTS): | .build/chainlint
+$(BUILT_CHAINLINTTESTS): .build/%.actual: %
+ $(CHAINLINT) <$< | \
+ sed -e '/^# LINT: /d' >$@ && \
+ diff -u $(basename $<).expect $@
+
+check-chainlint: $(BUILT_CHAINLINTTESTS)
test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
test-lint-filenames
--
2.34.1.1025.g9a0c3a30920
