From e01132227dfcbb6216cdeeb40212daba7d35499e Mon Sep 17 00:00:00 2001 From: "zhangheng.023" Date: Tue, 7 Jul 2026 12:28:54 +0800 Subject: [PATCH] fix: retry TempDir cleanup for test-created git repos Local git tooling that hooks trace2 (e.g. a global trace2.eventtarget socket listener) can asynchronously write into a repo's .git/ shortly after a git command runs. This races with t.TempDir's automatic RemoveAll cleanup and intermittently fails tests with "directory not empty", unrelated to the code under test. --- internal/qualitygate/diff/diff_test.go | 24 +++++++++++-- .../qualitygate/publiccontent/collect_test.go | 34 +++++++++++++++---- internal/qualitygate/rules/run_test.go | 34 +++++++++++++++---- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/internal/qualitygate/diff/diff_test.go b/internal/qualitygate/diff/diff_test.go index 71b117301..b631b8a9d 100644 --- a/internal/qualitygate/diff/diff_test.go +++ b/internal/qualitygate/diff/diff_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "testing" + "time" ) func TestScopeIncludesChangedSkillAndRelatedDomain(t *testing.T) { @@ -51,7 +52,7 @@ func TestFileAtRevisionMissingClassifier(t *testing.T) { } func TestChangedFilesIncludingWorktree(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -83,7 +84,7 @@ func TestChangedFilesIncludingWorktree(t *testing.T) { } func TestChangedFilesHandlesWhitespacePaths(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -139,3 +140,22 @@ func gitOutput(t *testing.T, repo string, args ...string) string { } return string(out[:len(out)-1]) } + +// newGitTestRepo returns a temp dir for a test-created git repo. Git tooling +// on this machine (trace2 hooks, etc.) can asynchronously write into a +// repo's .git/ shortly after a git command runs, racing with t.TempDir's +// automatic RemoveAll cleanup. Removing the tree ourselves first (retrying +// past that transient window) makes the later t.TempDir cleanup a no-op. +func newGitTestRepo(t *testing.T) string { + t.Helper() + repo := t.TempDir() + t.Cleanup(func() { + for i := 0; i < 10; i++ { + if err := os.RemoveAll(repo); err == nil { + return + } + time.Sleep(50 * time.Millisecond) + } + }) + return repo +} diff --git a/internal/qualitygate/publiccontent/collect_test.go b/internal/qualitygate/publiccontent/collect_test.go index 5ea92779f..b3cc24bcc 100644 --- a/internal/qualitygate/publiccontent/collect_test.go +++ b/internal/qualitygate/publiccontent/collect_test.go @@ -10,10 +10,11 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestCollectScansOnlyCurrentContributionAndMetadata(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -60,7 +61,7 @@ api_`+`key = "example-public-key" } func TestCollectScansOnlyChangedLinesInChangedFiles(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -92,7 +93,7 @@ func TestCollectScansOnlyChangedLinesInChangedFiles(t *testing.T) { } func TestCollectSemanticCandidatesStoreSanitizedReviewText(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -608,7 +609,7 @@ func TestCollectIgnoresDeletedPrivateKeyLine(t *testing.T) { } func TestCollectSkipsOnlyKnownQualityGateFixtureFiles(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -699,7 +700,7 @@ func TestCollectScansAddedLinesInSpecialPathNames(t *testing.T) { } func TestCollectScansBranchNameAsWarning(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) metadataPath := filepath.Join(repo, "pr-metadata.json") writeFile(t, metadataPath, `{"branch":"bot/public-doc-update"}`) got, err := Collect(context.Background(), Options{ @@ -799,7 +800,7 @@ func TestAppendUniqueFindingsDeduplicatesByRuleFileLineAndSource(t *testing.T) { func newGitRepo(t *testing.T) string { t.Helper() - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -840,7 +841,7 @@ func requireFinding(t *testing.T, got []Finding, file, rule string) { } func TestCollectRequiresValidMetadataJSON(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) metadataPath := filepath.Join(repo, "pr-metadata.json") writeFile(t, metadataPath, `{"title":`) @@ -874,6 +875,25 @@ func runGitOutput(t *testing.T, repo string, args ...string) []byte { return out } +// newGitTestRepo returns a temp dir for a test-created git repo. Git tooling +// on this machine (trace2 hooks, etc.) can asynchronously write into a +// repo's .git/ shortly after a git command runs, racing with t.TempDir's +// automatic RemoveAll cleanup. Removing the tree ourselves first (retrying +// past that transient window) makes the later t.TempDir cleanup a no-op. +func newGitTestRepo(t *testing.T) string { + t.Helper() + repo := t.TempDir() + t.Cleanup(func() { + for i := 0; i < 10; i++ { + if err := os.RemoveAll(repo); err == nil { + return + } + time.Sleep(50 * time.Millisecond) + } + }) + return repo +} + func writeFile(t *testing.T, path, data string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { diff --git a/internal/qualitygate/rules/run_test.go b/internal/qualitygate/rules/run_test.go index b60a2c632..a27d057dd 100644 --- a/internal/qualitygate/rules/run_test.go +++ b/internal/qualitygate/rules/run_test.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" "testing" + "time" qdiff "github.com/larksuite/cli/internal/qualitygate/diff" "github.com/larksuite/cli/internal/qualitygate/manifest" @@ -69,7 +70,7 @@ func TestReferenceCommandSurfaceNormalizesShortcutDomain(t *testing.T) { } func TestRunRequiresCommandIndexToCoverManifest(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) manifestPath := filepath.Join(repo, "command-manifest.json") indexPath := filepath.Join(repo, "command-index.json") m := manifest.Manifest{SchemaVersion: 1, Commands: []manifest.Command{{ @@ -104,7 +105,7 @@ func TestRunRequiresCommandIndexToCoverManifest(t *testing.T) { } func TestRunReadsManifestFilesAndAcceptsServiceReferences(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -190,7 +191,7 @@ description: Manage Drive comments with service command references. } func TestRunCollectsPublicContentFindingsIntoDiagnosticsAndFacts(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -283,7 +284,7 @@ func TestRunCollectsPublicContentFindingsIntoDiagnosticsAndFacts(t *testing.T) { } func TestLoadBaseReferenceManifestReadsCommandGolden(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -323,7 +324,7 @@ func TestLoadBaseReferenceManifestReadsCommandGolden(t *testing.T) { } func TestLoadBaseReferenceManifestReadsCommandIndexGolden(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -364,7 +365,7 @@ func TestLoadBaseReferenceManifestReadsCommandIndexGolden(t *testing.T) { } func TestLoadBaseReferenceManifestRejectsEmptyGolden(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -384,7 +385,7 @@ func TestLoadBaseReferenceManifestRejectsEmptyGolden(t *testing.T) { } func TestLoadBaseReferenceManifestRejectsInvalidGoldenKind(t *testing.T) { - repo := t.TempDir() + repo := newGitTestRepo(t) runGit(t, repo, "init") runGit(t, repo, "config", "user.email", "test@example.com") runGit(t, repo, "config", "user.name", "Test User") @@ -606,3 +607,22 @@ func runGit(t *testing.T, repo string, args ...string) { t.Fatalf("git %v failed: %v\n%s", args, err, out) } } + +// newGitTestRepo returns a temp dir for a test-created git repo. Git tooling +// on this machine (trace2 hooks, etc.) can asynchronously write into a +// repo's .git/ shortly after a git command runs, racing with t.TempDir's +// automatic RemoveAll cleanup. Removing the tree ourselves first (retrying +// past that transient window) makes the later t.TempDir cleanup a no-op. +func newGitTestRepo(t *testing.T) string { + t.Helper() + repo := t.TempDir() + t.Cleanup(func() { + for i := 0; i < 10; i++ { + if err := os.RemoveAll(repo); err == nil { + return + } + time.Sleep(50 * time.Millisecond) + } + }) + return repo +}