Subject: Re: [RFC PATCH 2/4] test-tool run-command testsuite: support unit tests

From: Junio C Hamano <[email-masked]>
Date: Tue, 16 Jan 2024 15:18:29 -0800

Josh Steadmon <[email-masked]> writes:

> Teach the testsuite runner in `test-tool run-command testsuite` how to
> run unit tests, by adding two new flags:
>
> First, "--(no-)run-in-shell" allows the test-tool to exec the unit-test
> binaries directly, rather than trying to interpret them as shell
> scripts.

Makes perfect sense.

> Second "--(no-)require-shell-test-pattern" bypasses the check that the
> test filenames match the expected t####-*.sh pattern.

This one I am not so sure.  Do we still have situations where
erroring out when fed a non t[0-9][0-9][0-9][0-9]-*.sh script is
problematic?  IOW, do we need to keep it as conditional?

	... goes and looks ...

Ah, this variable/option is misnamed and that is what invited the
above nonsense question out of me.  The logic this option disables
does not "require" (and error out if the requirement is not met); it
is used in a loop over "ls *" and "filtering" files out that are not
the numbered scripts.

But that confusion makes me wonder if non-script side of tests would
also want some filtering in the longer run, even if the directory we
feed to "test-tool run" happens to contain nothing but what we want
to run right now.  I wonder if we instead want a variable that holds
a pattern used to match programs readdir() discovers and skip those
that do not match the pattern?  Its default value may be something
like "t[0-9][0-9][0-9][0-9]-*.sh" but alternatively you can give,
say, "*" to pass everything, or something like that.

> With these changes, you can now use test-tool to run the unit tests:
> $ make
> $ cd t/unit-tests/bin
> $ ../../helper/test-tool run-command testsuite --no-run-in-shell \
>     --no-require-shell-test-pattern

This makes me wonder why we want to do the readdir() loop ourselves.
Instead of saying --no-require-shell-test-pattern there, wouldn't it
be simpler to say "*" right there, and have testsuite() run the test
programs named from the command line?

But that is orthogonal to the enhancement we have here.