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

From: Jeff King <peff@peff.net>
Date: Mar 4, 2025, 07:30 UTC
Message-ID: <20250304073010.GC1283943@coredump.intra.peff.net>
In-Reply-To: <xmqqzfi2xl5q.fsf@gitster.g>

On Mon, Mar 03, 2025 at 06:07:29AM -0800, Junio C Hamano wrote:

Show 13 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Maybe. I guess for unit tests it's a lot less clear cut as most of the
> > tests won't depend on such a controlled environment. So sanitizing the
> > environment would be a good enough first step for me, and if we see
> > demand for making specific information available to lots of tests we
> > could still start to expose those at a later point.
> 
> Fair enough.
> 
> To put it another way, if you write a test and if it gets affected
> by externalities, perhaps you are testing a function that is at too
> high a level that is not a suitable target for unit tested?

I think one problem with this approach is that breakage is likely going to depend on the user's environment. So something that works just fine for you, the test author, may introduce a hidden dependency that breaks for somebody else much later.

Some examples, assuming we just suppress reading Git config:

  - Without an explicit ident, we fall back to constructing one from
    system info. So if a unit test ever creates a commit, it will work
    fine for most people, but not for somebody with a blank GECOS field
    in /etc/passwd. (We do look at that field for reflogs, which current
    unit tests already do, but we are more forgiving there since we
    don't pass IDENT_STRICT).

  - Other programs we call (e.g., imagine gpg or ssh for commit signing
    or verification) may read their own config based on $HOME,
    $XDG_CONFIG_HOME, etc. I don't know if Patrick was including that in
    "sanitizing the environment" or not.

-Peff