Re: [BUG/WIP PATCH] unit-tests: use clean test environment

[Patrick Steinhardt <ps@pks.im>]

On Tue, Feb 25, 2025 at 09:48:47PM +0100, Michael J Gruber wrote:
> So far, unit-tests are run in the user's environment, in particular with
> the user's git config which can influence test runs.
> 
> Set up the same clean test environment which we use for the other tests.
> ---
> Hi there,
> 
> I finally got around to checking whether the last test in t-trailer fails
> for me: one of the trailers is present in my git config, messing up the
> test assumption. The reasons is a systematic problem in the way we run
> unit-tests.
> 
> The attached patch is more POC than WIP - it makes t-trailer work again
> for me, at least.
> 
> unittest-lib.sh is a trimmed down version of test-lib.sh, and I'm sure
> it can be trimmed down even more, maybe to the extent of being included
> in run-test.sh.
> 
> Also, I'm not sute the Makefile change catches all invocations of
> unit-tests. (It certainly does not cover direct invocations, of course.)

This to me is the biggest issue with the chosen approach. It would be
great if we could find a way to sanitize the environment in the unit
test executable directly instead of forcing users to run the unit tests
via the provided script.

I suspect that most environment variables shouldn't matter (for now), so
overall the duplication may be acceptable:

  - We need to unset a couple of variables, but we can probably reuse
    logic `git rev-parse --local-env-vars`.

  - We need to ask Git to not read the configuration, which we can do by
    setting a couple of envvars. This should be manageable.

But other than that I don't think we rely on any other infrastructure
exposed via environment variables, to the best of my knowledge. So it
shouldn't be that much logic, which may make it an option to implement
this in "t/unit-tests/unit-test.c" directly.

We still have a couple of other unit tests that have a different entry
point than the clar-based tests. But Seyi has been converting most of
them already, and I wouldn't worry about the remaining ones. So only
doing it for the clar-based tests should be good enough for our case as
we can assume that the other tests will go away sooner rather than
later.

Thanks!

Patrick