git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 4, 2025, 07:39 UTC
Message-ID
<Z8auRqylLYuwalAA@pks.im>
In-Reply-To
<20250304073010.GC1283943@coredump.intra.peff.net>
On Tue, Mar 04, 2025 at 02:30:10AM -0500, Jeff King wrote:
Show 34 quoted lines
> On Mon, Mar 03, 2025 at 06:07:29AM -0800, Junio C Hamano wrote:
> 
> > 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.

Oh, yes. I didn't mean to say we shouldn't sanitize at all, I rather meant to say we should sanitize to values that simply cause us to do a no-op in the relevant parts. That means we'd:

    - Unset a bunch of environment variables where we know that they
      impact Git.
    - Set config-related environment variables to read configuration
      from "/dev/null".

This is in contrast to the more involved fix here, which would be to populate a temporary home directory with gitconfig files and whatnot.

Patrick
Previous: Jeff KingNext: Jeff King
Message 8 of 10 in “unit-tests: use clean test environment”
  1. unit-tests: use clean test environmentMichael J Gruber, Feb 25, 2025
  2. Patrick SteinhardtFeb 28, 2025
  3. Junio C HamanoFeb 28, 2025
  4. Patrick SteinhardtMar 3, 2025
  5. Michael J GruberMar 3, 2025
  6. Junio C HamanoMar 3, 2025
  7. Jeff KingMar 4, 2025
  8. Patrick SteinhardtMar 4, 2025
  9. Jeff KingMar 4, 2025
  10. Junio C HamanoMar 4, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.