Re: [PATCH 2/2] t7900: fix flaky "maintenance.strategy" test

On Wed, Aug 12, 2026 at 01:46:14AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <[email protected]> writes:
> 
> > One of our tests for whether "maintenance.strategy" is being respected
> > in t7900 is flaky in our CI systems:
> >
> >     + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet
> >     + test_maintenance_tasks trace2.txt
> >     + cat
> >     + sed -ne s/.*"region_enter".*"category":"maintenance\([^"]*\)".*"label":"\([^"][^"]*\)".*/\2\1/p trace2.txt
> >     + test_cmp expect actual
> >     + test 2 -ne 2
> >     + eval /usr/bin/diff -u "$@"
> >     + /usr/bin/diff -u expect actual
> >     --- expect	2026-08-07 06:20:51.388322602 +0000
> >     +++ actual	2026-08-07 06:20:51.388322602 +0000
> >     @@ -1,2 +0,0 @@
> >     -gc foreground
> >     -gc
> >
> > When running with the "incremental" strategy, we expect two git-gc(1)
> > tasks to have been executed, but sometimes the test simply doesn't
> > execute any of those tasks.
> >
> > A first hunch may be that maybe the disk-state is sometimes different
> > and thus we decide not to run maintenance. But git-maintenance(1)
> > doesn't run with the "--auto" switch, so we should execute those tasks
> > regardless of the on-disk state.
> >
> > But there's a second condition that may cause us to not execute tasks,
> > namely when the "maintenance.lock" file exists due to a concurrently
> 
> Nit: s/a//
> 
> > running tasks. We usually disable auto-maintenance from detaching in our
> > test suite to avoid exactly these kinds of race conditions, but in t7900
> > we unset "GIT_TEST_MAINT_AUTO_DETACH" and thus enable the auto-detach
> > logic. The intent of this is to exercise git-maintenance(1) closer to
> > how it would run in a real-world scenario, but it does cause us to race
> > when the detached maintenance job that was triggered by `test_commit()`
> > lives long enough.
> 
> GIT_TEST_MAINT_AUTO_DETACH when set to true enables auto-detach, but
> also the default value when unset is true. That's why unsetting it
> enables auto-detach. That's a bit confusing.

I'll reword this paragraph a bit. Thanks!

Patrick