Skip to content

Commit dd909ab

Browse files
author
Frank Guo
committed
Test: disable git auto-gc to fix TestPush_E2E_ExportAndPush flake
Root cause: git's receive-pack runs `git gc --auto` after a push once loose-object/pack thresholds are crossed, and that gc forks and detaches — control returns to the pushing `git push` (and so to the test) before gc necessarily finishes repacking/pruning the receiving repo. TestPush_E2E_ExportAndPush does two checkpoint→push cycles in quick succession, which is exactly the shape that crosses gc.auto's thresholds. t.TempDir()'s cleanup (os.RemoveAll) then raced that detached background process: the exact failure CI hit, "TempDir RemoveAll cleanup: unlinkat .../002: directory not empty", is git gc still touching the bare remote directory (the second t.TempDir() call in the test) when RemoveAll walked it moments later. Not a DuckDB/nomic handle leak or a missing Close() — every connection opened during the test's two checkpoint/push cycles was already correctly deferred-closed (checked doCheckpoint, push.go's exportNewFrames and markCheckpointsExported, and updateIndexIncremental). This is a test-lifecycle issue, not a product bug: a real user's repo is never subject to a test harness deleting it out from under a background gc a few milliseconds after a push returns. Fixed in the test harness: every git repo created in integration tests (the main TestEnv repo, every bare push remote, and clones used to simulate a teammate) now gets gc.auto=0 and receive.autogc=false set immediately after creation, via a new initBareRemote/disableAutoGC helper replacing four copy-pasted `git init --bare` blocks in checkpoint_e2e_test.go. Verified: TestPush_E2E_ExportAndPush at -count=50 -race (previously flaky under CI load) and the three push/import/conflict E2E tests together at -count=20 -race, all clean. Full suite + lint green.
1 parent 106af60 commit dd909ab

2 files changed

Lines changed: 57 additions & 12 deletions

File tree

cmd/rekal/cli/integration_test/checkpoint_e2e_test.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,7 @@ func TestPush_E2E_ExportAndPush(t *testing.T) {
208208
// Create bare remote.
209209
bareDir := t.TempDir()
210210
bareDir, _ = filepath.EvalSymlinks(bareDir)
211-
if err := exec.Command("git", "init", "--bare", bareDir).Run(); err != nil {
212-
t.Fatalf("git init --bare: %v", err)
213-
}
211+
initBareRemote(t, bareDir)
214212
if err := exec.Command("git", "-C", env.RepoDir, "remote", "add", "origin", bareDir).Run(); err != nil {
215213
t.Fatalf("git remote add: %v", err)
216214
}
@@ -384,9 +382,7 @@ func TestPush_E2E_ForceOnConflict(t *testing.T) {
384382

385383
bareDir := t.TempDir()
386384
bareDir, _ = filepath.EvalSymlinks(bareDir)
387-
if err := exec.Command("git", "init", "--bare", bareDir).Run(); err != nil {
388-
t.Fatalf("git init --bare: %v", err)
389-
}
385+
initBareRemote(t, bareDir)
390386
if err := exec.Command("git", "-C", env.RepoDir, "remote", "add", "origin", bareDir).Run(); err != nil {
391387
t.Fatalf("git remote add: %v", err)
392388
}
@@ -407,6 +403,7 @@ func TestPush_E2E_ForceOnConflict(t *testing.T) {
407403
for _, kv := range [][2]string{
408404
{"user.email", "other@rekal.dev"},
409405
{"user.name", "Other User"},
406+
{"gc.auto", "0"},
410407
} {
411408
exec.Command("git", "-C", cloneDir, "config", kv[0], kv[1]).Run()
412409
}
@@ -568,9 +565,7 @@ func TestImport_E2E_RoundTrip(t *testing.T) {
568565
// Create bare remote and push.
569566
bareDir := t.TempDir()
570567
bareDir, _ = filepath.EvalSymlinks(bareDir)
571-
if err := exec.Command("git", "init", "--bare", bareDir).Run(); err != nil {
572-
t.Fatalf("git init --bare: %v", err)
573-
}
568+
initBareRemote(t, bareDir)
574569
if err := exec.Command("git", "-C", env.RepoDir, "remote", "add", "origin", bareDir).Run(); err != nil {
575570
t.Fatalf("git remote add: %v", err)
576571
}
@@ -596,6 +591,7 @@ func TestImport_E2E_RoundTrip(t *testing.T) {
596591
for _, kv := range [][2]string{
597592
{"user.email", "test@rekal.dev"},
598593
{"user.name", "Test User"},
594+
{"gc.auto", "0"},
599595
} {
600596
exec.Command("git", "-C", cloneDir, "config", kv[0], kv[1]).Run()
601597
}
@@ -749,9 +745,7 @@ func TestPush_NoNewCheckpoints(t *testing.T) {
749745

750746
bareDir := t.TempDir()
751747
bareDir, _ = filepath.EvalSymlinks(bareDir)
752-
if err := exec.Command("git", "init", "--bare", bareDir).Run(); err != nil {
753-
t.Fatalf("git init --bare: %v", err)
754-
}
748+
initBareRemote(t, bareDir)
755749
if err := exec.Command("git", "-C", env.RepoDir, "remote", "add", "origin", bareDir).Run(); err != nil {
756750
t.Fatalf("git remote add: %v", err)
757751
}

cmd/rekal/cli/integration_test/commands_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,14 @@ func NewTestEnv(t *testing.T) *TestEnv {
3838
t.Fatalf("git init: %v", err)
3939
}
4040
// Set local git user config (required for git commit-tree in ensureOrphanBranch).
41+
// gc.auto=0/receive.autogc=false: git forks a detached `git gc --auto`
42+
// after commits/pushes once loose-object thresholds are crossed — see
43+
// disableAutoGC's doc comment for why that matters for a t.TempDir().
4144
for _, kv := range [][2]string{
4245
{"user.email", "test@rekal.dev"},
4346
{"user.name", "Rekal Test"},
47+
{"gc.auto", "0"},
48+
{"receive.autogc", "false"},
4449
} {
4550
c := exec.Command("git", "-C", dir, "config", kv[0], kv[1])
4651
if err := c.Run(); err != nil {
@@ -50,6 +55,52 @@ func NewTestEnv(t *testing.T) *TestEnv {
5055
return &TestEnv{T: t, RepoDir: dir}
5156
}
5257

58+
// initBareRemote creates a bare git repo at dir (for use as a `git push`
59+
// remote in tests) with automatic gc disabled — see disableAutoGC.
60+
func initBareRemote(t *testing.T, dir string) {
61+
t.Helper()
62+
if err := exec.Command("git", "init", "--bare", dir).Run(); err != nil {
63+
t.Fatalf("git init --bare %s: %v", dir, err)
64+
}
65+
disableAutoGC(t, dir)
66+
}
67+
68+
// disableAutoGC turns off git's automatic background maintenance for a repo
69+
// used in a test.
70+
//
71+
// This is the fix for a real, reproduced flake in TestPush_E2E_ExportAndPush:
72+
// receive-pack runs `git gc --auto` after a push once loose-object/pack
73+
// thresholds are crossed, and that gc forks and detaches — control returns to
74+
// the pushing `git push` (and so to RunCLI, and so to the test) before gc
75+
// necessarily finishes repacking/pruning the receiving repo's objects. A test
76+
// that pushes more than once in quick succession (this one does two
77+
// checkpoint→push cycles) is exactly the shape that crosses gc.auto's
78+
// thresholds. t.TempDir()'s cleanup (os.RemoveAll) then races that detached
79+
// process: if gc is still writing/removing pack or loose-object files when
80+
// RemoveAll walks the directory, RemoveAll can fail with ENOTEMPTY
81+
// ("directory not empty") because a file it already listed gets
82+
// removed/replaced out from under it, or a new file appears after it thought
83+
// the directory was empty. That is the exact failure signature CI hit:
84+
// "TempDir RemoveAll cleanup: unlinkat .../002: directory not empty" — path
85+
// "002" being the bare remote directory, the second t.TempDir() call in that
86+
// test.
87+
//
88+
// This is a test-lifecycle issue, not a product bug: a real user's repos are
89+
// never subject to a test harness deleting them out from under a background
90+
// gc a few milliseconds after a push returns. The fix belongs here, not in
91+
// rekal's own git invocations.
92+
func disableAutoGC(t *testing.T, dir string) {
93+
t.Helper()
94+
for _, kv := range [][2]string{
95+
{"gc.auto", "0"},
96+
{"receive.autogc", "false"},
97+
} {
98+
if err := exec.Command("git", "-C", dir, "config", kv[0], kv[1]).Run(); err != nil {
99+
t.Fatalf("git -C %s config %s %s: %v", dir, kv[0], kv[1], err)
100+
}
101+
}
102+
}
103+
53104
// NewTestEnvAt creates a TestEnv pointing at an existing git repo directory.
54105
func NewTestEnvAt(t *testing.T, dir string) *TestEnv {
55106
t.Helper()

0 commit comments

Comments
 (0)