Re: [PATCH v2 1/2] pretty: update tests to use `test_config`

From: Jeff King <peff@peff.net>
Date: Mar 25, 2024, 09:44 UTC
Message-ID: <20240325094444.GA254602@coredump.intra.peff.net>
In-Reply-To: <20240325072651.947505-1-brianmlyles@gmail.com>

On Mon, Mar 25, 2024 at 02:25:12AM -0500, Brian Lyles wrote:

Show 7 quoted lines

> These tests use raw `git config` calls, which is an older style that can
> cause config to bleed between tests if not manually unset. `test_config`
> ensures that config is unset at the end of each test automatically.
> 
> `test_config` is chosen over `git -c` since `test_config` still ends up
> calling `git config` which seems slightly more realistic to how pretty
> formats would be defined normally.

I think what you have here is fine, and I agree it's more like what would happen in practice. The nice thing about "-c" is that it involves two fewer processes (one to make the config and one to clean up). But that is really the tip of the iceberg for our test suite.

>  t/t4205-log-pretty-formats.sh | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)

The patch looks good to me. The big thing to be careful about here is tests which actually require the config state to persist between hunks. But none of these look to be that type.

-Peff