t: fix races caused by background maintenance

Many Git commands spawn git-maintenance(1) to optimize the repository in
the background. By default, performing the maintenance is for most of
the part asynchronous: we fork the executable and then continue with the
rest of our business logic.

This is working as expected for our users, but this behaviour is
somewhat problematic for our test suite as this is inherently racy. We
have many tests that verify the on-disk state of repositories, and those
tests may easily race with our background maintenance. In a similar
fashion, we may end up with processes that "leak" out of a current test
case.

Until now this tends to not be much of a problem. Our maintenance uses
git-gc(1) by default, which knows to bail out in case there aren't
either too many packfiles or too many loose objects. So even if other
data structures would need to be optimized, we won't do so unless the
object database also needs optimizations.

This is about to change though, as a subsequent commit will switch to
the "geometric" maintenance strategy as a default. The consequence is
that we will run required optimizations even if the object database is
well-optimized. And this uncovers races between our test suite and
background maintenance all over the place.

Disabling maintenance outright in our test suite is not really an
option, as it would result in significant divergence from the "real
world" and reduce our test coverage. But we've got an alternative up our
sleeves: we can ensure that garbage collection runs synchronously by
overriding the "maintenance.autoDetach" configuration.

Of course that also diverges from the real world, as we now stop testing
that background maintenance interacts in a benign way with normal Git
commands. But on the other hand this ensures that the maintenance itself
does not for example lead to data loss in a more reproducible way.

Another concern is that this would make execution of the test suite much
slower. But a quick benchmark on my machine demonstrates that this does
not seem to be the case:

    Benchmark 1: meson test (revision = HEAD~)
      Time (mean ± σ):     131.182 s ±  1.293 s    [User: 853.737 s, System: 1160.479 s]
      Range (min … max):   130.001 s … 132.563 s    3 runs

    Benchmark 2: meson test (revision = HEAD)
      Time (mean ± σ):     129.554 s ±  0.507 s    [User: 849.040 s, System: 1152.664 s]
      Range (min … max):   129.000 s … 129.994 s    3 runs

    Summary
      meson test (revision = HEAD) ran
        1.01 ± 0.01 times faster than meson test (revision = HEAD~)

Funny enough, it even seems as if this speeds up test execution ever so
slightly, but that may just as well be noise.

Introduce a new `GIT_TEST_MAINT_AUTO_DETACH` environment variable that
allows us to override the auto-detach behaviour and set that variable in
our tests.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/run-command.c b/run-command.c
index e3e02475ccec50163865d571902b18ba2e339a36..438a290d30744d88dfc2548c8d74cf8fd7435a4f 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -1828,7 +1828,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
         */
        if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) &&
            repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach))
-               auto_detach = 1;
+               auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
        maint->git_cmd = 1;
        maint->close_object_store = 1;

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 1e354e057fa12ce354c8146bd3920b380d97a08e..d62760eb92b42228c2311fe7fdc868cc9e174286 100755 (executable)
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -229,7 +229,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
 
        GIT_TRACE2_EVENT="$PWD/trace1.event" \
        git -C pc1 fetch --refetch origin &&
-       test_subcommand git maintenance run --auto --no-quiet --detach <trace1.event &&
+       test_subcommand git maintenance run --auto --no-quiet --no-detach <trace1.event &&
        grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace1.event &&
        grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace1.event &&
 
@@ -238,7 +238,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
                -c gc.autoPackLimit=0 \
                -c maintenance.incremental-repack.auto=1234 \
                -C pc1 fetch --refetch origin &&
-       test_subcommand git maintenance run --auto --no-quiet --detach <trace2.event &&
+       test_subcommand git maintenance run --auto --no-quiet --no-detach <trace2.event &&
        grep \"param\":\"gc.autopacklimit\",\"value\":\"0\" trace2.event &&
        grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"-1\" trace2.event &&
 
@@ -247,7 +247,7 @@ test_expect_success 'fetch --refetch triggers repacking' '
                -c gc.autoPackLimit=1234 \
                -c maintenance.incremental-repack.auto=0 \
                -C pc1 fetch --refetch origin &&
-       test_subcommand git maintenance run --auto --no-quiet --detach <trace3.event &&
+       test_subcommand git maintenance run --auto --no-quiet --no-detach <trace3.event &&
        grep \"param\":\"gc.autopacklimit\",\"value\":\"1\" trace3.event &&
        grep \"param\":\"maintenance.incremental-repack.auto\",\"value\":\"0\" trace3.event
 '

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 7cc0ce57f8f32088d0d575c58127e5d76af029ff..fe344f47ee5c89a8586ed6393d4624dc5d935cbf 100755 (executable)
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -7,6 +7,9 @@ test_description='git maintenance builtin'
 GIT_TEST_COMMIT_GRAPH=0
 GIT_TEST_MULTI_PACK_INDEX=0
 
+# Ensure that auto-maintenance detaches as usual.
+sane_unset GIT_TEST_MAINT_AUTO_DETACH
+
 test_lazy_prereq XMLLINT '
        xmllint --version
 '

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fb76f7d11e840708d5db521d773b606f2b8bd79..aa805a01ce6035aa1c09894b57abeb86dd47184a 100644 (file)
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1947,6 +1947,10 @@ test_lazy_prereq COMPAT_HASH '
 GIT_TEST_MAINT_SCHEDULER="none:exit 1"
 export GIT_TEST_MAINT_SCHEDULER
 
+# Ensure that tests cannot race with background maintenance by default.
+GIT_TEST_MAINT_AUTO_DETACH="false"
+export GIT_TEST_MAINT_AUTO_DETACH
+
 # Does this platform support `git fsmonitor--daemon`
 #
 test_lazy_prereq FSMONITOR_DAEMON '