Subject: Re: [RFC PATCH v2 2/6] test-tool run-command testsuite: get shell from env

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Feb 2024 12:55:30 -0800

Josh Steadmon <[email-masked]> writes:

> When running tests through `test-tool run-command testsuite`, we
> currently hardcode `sh` as the command interpreter. As discussed in [1],
> this is incorrect, and we should be using the shell set in
> TEST_SHELL_PATH instead.
>
> Add a shell_path field in struct testsuite so that we can pass this to
> the task runner callback. If this is non-null, we'll use it as the
> argv[0] of the subprocess. Otherwise, we'll just execute the test
> program directly.

That sounds nice in theory, but ...

> When setting up the struct testsuite in testsuite(), use the value
> of TEST_SHELL_PATH if it's set, otherwise default to `sh`.

... this done in the testsuite() function, doesn't suite.shell_path
always gets some non-NULL value?  Perhaps in a later step we will
add a mechanism to allow suite.shell_path to be NULL when we know
we are running an executable, or something?

Leaving readers in a bit of suspense may, especially in a series
that is short like this, be a good technique to entice them to keep
reading, perhaps, but anyway, if that is what is intended, a simple
"this feature is not used in this step, but we will take advantage
of it soon in a later step" would be a good idea.