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

From: Jeff King <peff@peff.net>
Date: Jan 23, 2024, 00:59 UTC
Message-ID: <20240123005913.GB835964@coredump.intra.peff.net>
In-Reply-To: <xmqqil3sx2y6.fsf@gitster.g>

On Tue, Jan 16, 2024 at 03:40:01PM -0800, Junio C Hamano wrote:

Show 16 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Josh Steadmon <steadmon@google.com> 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