Subject: Re: [PATCH v8 04/17] scalar: create test infrastructure

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 30 Nov 2021 14:27:03 +0100

On Fri, Nov 19 2021, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[email-masked]>
> [...]
> +-include ../../../config.mak.autogen
> +-include ../../../config.mak
> +
> +SHELL_PATH ?= $(SHELL)
> +PERL_PATH ?= /usr/bin/perl
> +RM ?= rm -f
> +PROVE ?= prove
> +DEFAULT_TEST_TARGET ?= test
> +TEST_LINT ?= test-lint
> +
> +ifdef TEST_OUTPUT_DIRECTORY
> +TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
> +else
> +TEST_RESULTS_DIRECTORY = ../../../t/test-results
> +endif

So here we'll inject our output into t/test-results[...]

> +T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))

...end up with a $(T) with just the one scalar test...

> +test-lint-duplicates:
> +	@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
> +		test -z "$$dups" || { \
> +		echo >&2 "duplicate test numbers:" $$dups; exit 1; }
> +

...so I *think* (but haven't checked) that print-test-failures.sh will
work with the later CI integration, but both t/Makefile & this one
assume in some ways that test numbers are unique, this now lives in the
same namespace, but neither check the two for conflicts.

We only have the one t9099 test here, so unless we add another one in
the top-level t/ that should be OK, but is there any reason to carry
this at all? It seems pointless given the above.

I think to make this meaningful we'd need to teach t/Makefile to have a
T_ALL variable or something, derived from its T, and have that wildcard
the scalar tests and other contrib tests and check them for namespacing
conflicts.

> +test-results:
> +	mkdir -p test-results
> +

This target seems to be unused, don't we always use the top-level
t/test-results? This seems to be copy/pasting also dead code from
contrib/subtree/t/Makefile.