Re: [PATCH 2/3] t/test-tool: handle -c <name>=<value> arguments

[Taylor Blau <me@ttaylorr.com>]

On Tue, Jan 09, 2024 at 04:29:57PM +0100, Rubén Justo wrote:
> Soon we're going to need to pass configuration values to a command in
> test-tool.
>
> Let's teach test-tool to take config values via command line arguments.

I wasn't expecting a step like this to appear in this series. I don't
have strong feelings about it, especially since test-tool helpers
already understand $GIT_DIR/config when they rely on library code which
implicitly reads configuration.

But it does seem odd to have test-tool invocations that intimately
depend on a particular set of configuration values. At the very least,
this step seems to encourage passing finely tuned configuration values
to test-tool helpers, which I am not sure is a good idea.

Your patch message suggests that this will be useful in the following
patch, which makes sense. But I wonder if it would be easier to avoid
the test-tool entirely and call some Git command in a state that we
expect to generate advice. Then we can test its output with various
values of advice.adviceOff.

> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
>  t/helper/test-tool.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

The patch itself looks reasonable, though.

Thanks,
Taylor