On Tue, Jan 16, 2024 at 03:40:01PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > Josh Steadmon <steadmon@xxxxxxxxxx> 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.
>
> This may be a stupid question, but do we even need the current "push
> 'sh' to the strvec"? If our executable shell scripts run just fine
> without, then this may not have to be conditional.
It is necessary for the same reason that the indirection provided by
patch 4 is necessary: the test scripts are supposed to be run by
TEST_SHELL_PATH, even if they may say "#!/bin/sh" in their header.
But the "testsuite" helper just hard-codes "sh", which is wrong. It's
somewhat academic for Windows where "sh" is already bash, and that's the
only reasonable thing anybody would set TEST_SHELL_PATH to anyway. But
if the point is to be a drop-in replacement for the existing flow, it
should be checking getenv("TEST_SHELL_PATH"). And perhaps the Makefile
might need to correctly export the variable.
-Peff
