fix(worktree): Cache git detection results to eliminate slowness (bd-7di)
In git worktrees, any bd command was slow with a 2-3s pause because: - git.IsWorktree() was called 4+ times per command - Each call spawned 2 git processes (git-dir and git-common-dir) - git.GetRepoRoot() and git.GetMainRepoRoot() also called multiple times Fix: Cache results using sync.Once since these values do not change during a single command execution: - IsWorktree() - caches worktree detection - GetRepoRoot() - caches repo root path - GetMainRepoRoot() - caches main repo root for worktrees Added ResetCaches() for test cleanup between subtests that change directories. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,8 @@ import (
|
|||||||
"os/exec"
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/steveyegge/beads/internal/git"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestFindDatabasePathEnvVar(t *testing.T) {
|
func TestFindDatabasePathEnvVar(t *testing.T) {
|
||||||
@@ -821,6 +823,7 @@ func TestFindGitRoot_RegularRepo(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
t.Chdir(subDir)
|
t.Chdir(subDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// findGitRoot should return the repo root
|
// findGitRoot should return the repo root
|
||||||
result := findGitRoot()
|
result := findGitRoot()
|
||||||
@@ -897,6 +900,7 @@ func TestFindGitRoot_Worktree(t *testing.T) {
|
|||||||
|
|
||||||
// Change to the worktree directory
|
// Change to the worktree directory
|
||||||
t.Chdir(worktreeDir)
|
t.Chdir(worktreeDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// findGitRoot should return the WORKTREE root, not the main repo root
|
// findGitRoot should return the WORKTREE root, not the main repo root
|
||||||
result := findGitRoot()
|
result := findGitRoot()
|
||||||
@@ -927,6 +931,7 @@ func TestFindGitRoot_NotGitRepo(t *testing.T) {
|
|||||||
defer os.RemoveAll(tmpDir)
|
defer os.RemoveAll(tmpDir)
|
||||||
|
|
||||||
t.Chdir(tmpDir)
|
t.Chdir(tmpDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// findGitRoot should return empty string
|
// findGitRoot should return empty string
|
||||||
result := findGitRoot()
|
result := findGitRoot()
|
||||||
@@ -1024,6 +1029,7 @@ func TestFindBeadsDir_Worktree(t *testing.T) {
|
|||||||
|
|
||||||
// Change to worktree
|
// Change to worktree
|
||||||
t.Chdir(worktreeDir)
|
t.Chdir(worktreeDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// FindBeadsDir should prioritize the main repo's .beads for worktrees (bd-de6)
|
// FindBeadsDir should prioritize the main repo's .beads for worktrees (bd-de6)
|
||||||
result := FindBeadsDir()
|
result := FindBeadsDir()
|
||||||
@@ -1134,6 +1140,7 @@ func TestFindDatabasePath_Worktree(t *testing.T) {
|
|||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
t.Chdir(worktreeSubDir)
|
t.Chdir(worktreeSubDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// FindDatabasePath should find the main repo's shared database
|
// FindDatabasePath should find the main repo's shared database
|
||||||
result := FindDatabasePath()
|
result := FindDatabasePath()
|
||||||
@@ -1241,6 +1248,7 @@ func TestFindDatabasePath_WorktreeNoLocalDB(t *testing.T) {
|
|||||||
|
|
||||||
// Change to worktree
|
// Change to worktree
|
||||||
t.Chdir(worktreeDir)
|
t.Chdir(worktreeDir)
|
||||||
|
git.ResetCaches() // Reset after chdir for caching tests
|
||||||
|
|
||||||
// FindDatabasePath should find the main repo's shared database
|
// FindDatabasePath should find the main repo's shared database
|
||||||
result := FindDatabasePath()
|
result := FindDatabasePath()
|
||||||
|
|||||||
@@ -93,6 +93,13 @@ func isWorktreeUncached() bool {
|
|||||||
return absGit != absCommon
|
return absGit != absCommon
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// mainRepoRootOnce ensures we only get main repo root once per process.
|
||||||
|
var (
|
||||||
|
mainRepoRootOnce sync.Once
|
||||||
|
mainRepoRootResult string
|
||||||
|
mainRepoRootErr error
|
||||||
|
)
|
||||||
|
|
||||||
// GetMainRepoRoot returns the main repository root directory.
|
// GetMainRepoRoot returns the main repository root directory.
|
||||||
// When in a worktree, this returns the main repository root.
|
// When in a worktree, this returns the main repository root.
|
||||||
// Otherwise, it returns the regular repository root.
|
// Otherwise, it returns the regular repository root.
|
||||||
@@ -101,7 +108,16 @@ func isWorktreeUncached() bool {
|
|||||||
// /project/.worktrees/feature/), this correctly returns the main repo
|
// /project/.worktrees/feature/), this correctly returns the main repo
|
||||||
// root (/project/) by using git rev-parse --git-common-dir which always
|
// root (/project/) by using git rev-parse --git-common-dir which always
|
||||||
// points to the main repo's .git directory. (GH#509)
|
// points to the main repo's .git directory. (GH#509)
|
||||||
|
// The result is cached after the first call.
|
||||||
func GetMainRepoRoot() (string, error) {
|
func GetMainRepoRoot() (string, error) {
|
||||||
|
mainRepoRootOnce.Do(func() {
|
||||||
|
mainRepoRootResult, mainRepoRootErr = getMainRepoRootUncached()
|
||||||
|
})
|
||||||
|
return mainRepoRootResult, mainRepoRootErr
|
||||||
|
}
|
||||||
|
|
||||||
|
// getMainRepoRootUncached performs the actual main repo root lookup without caching.
|
||||||
|
func getMainRepoRootUncached() (string, error) {
|
||||||
// Use --git-common-dir which always returns the main repo's .git directory,
|
// Use --git-common-dir which always returns the main repo's .git directory,
|
||||||
// even when running from within a worktree or its subdirectories.
|
// even when running from within a worktree or its subdirectories.
|
||||||
// This is the most reliable method for finding the main repo root.
|
// This is the most reliable method for finding the main repo root.
|
||||||
@@ -191,3 +207,17 @@ func getGitDirNoError(flag string) string {
|
|||||||
}
|
}
|
||||||
return strings.TrimSpace(string(out))
|
return strings.TrimSpace(string(out))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ResetCaches resets all cached git information. This is intended for use
|
||||||
|
// by tests that need to change directory between subtests.
|
||||||
|
// In production, these caches are safe because the working directory
|
||||||
|
// doesn't change during a single command execution.
|
||||||
|
func ResetCaches() {
|
||||||
|
isWorktreeOnce = sync.Once{}
|
||||||
|
isWorktreeResult = false
|
||||||
|
mainRepoRootOnce = sync.Once{}
|
||||||
|
mainRepoRootResult = ""
|
||||||
|
mainRepoRootErr = nil
|
||||||
|
repoRootOnce = sync.Once{}
|
||||||
|
repoRootResult = ""
|
||||||
|
}
|
||||||
@@ -864,6 +864,7 @@ func TestCountJSONLIssues(t *testing.T) {
|
|||||||
// TestGetMainRepoRoot tests the GetMainRepoRoot function for various scenarios
|
// TestGetMainRepoRoot tests the GetMainRepoRoot function for various scenarios
|
||||||
func TestGetMainRepoRoot(t *testing.T) {
|
func TestGetMainRepoRoot(t *testing.T) {
|
||||||
t.Run("returns correct root for regular repo", func(t *testing.T) {
|
t.Run("returns correct root for regular repo", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -877,6 +878,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
if err := os.Chdir(repoPath); err != nil {
|
if err := os.Chdir(repoPath); err != nil {
|
||||||
t.Fatalf("Failed to chdir to repo: %v", err)
|
t.Fatalf("Failed to chdir to repo: %v", err)
|
||||||
}
|
}
|
||||||
|
ResetCaches() // Reset after chdir
|
||||||
|
|
||||||
root, err := GetMainRepoRoot()
|
root, err := GetMainRepoRoot()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -893,6 +895,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("returns main repo root from worktree", func(t *testing.T) {
|
t.Run("returns main repo root from worktree", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -913,6 +916,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
if err := os.Chdir(worktreePath); err != nil {
|
if err := os.Chdir(worktreePath); err != nil {
|
||||||
t.Fatalf("Failed to chdir to worktree: %v", err)
|
t.Fatalf("Failed to chdir to worktree: %v", err)
|
||||||
}
|
}
|
||||||
|
ResetCaches() // Reset after chdir
|
||||||
|
|
||||||
root, err := GetMainRepoRoot()
|
root, err := GetMainRepoRoot()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -929,6 +933,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("returns main repo root from nested worktree (GH#509)", func(t *testing.T) {
|
t.Run("returns main repo root from nested worktree (GH#509)", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -950,6 +955,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
if err := os.Chdir(nestedWorktreePath); err != nil {
|
if err := os.Chdir(nestedWorktreePath); err != nil {
|
||||||
t.Fatalf("Failed to chdir to nested worktree: %v", err)
|
t.Fatalf("Failed to chdir to nested worktree: %v", err)
|
||||||
}
|
}
|
||||||
|
ResetCaches() // Reset after chdir
|
||||||
|
|
||||||
root, err := GetMainRepoRoot()
|
root, err := GetMainRepoRoot()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -966,6 +972,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("returns main repo root from subdirectory of nested worktree", func(t *testing.T) {
|
t.Run("returns main repo root from subdirectory of nested worktree", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -993,6 +1000,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
if err := os.Chdir(subDir); err != nil {
|
if err := os.Chdir(subDir); err != nil {
|
||||||
t.Fatalf("Failed to chdir to subdir: %v", err)
|
t.Fatalf("Failed to chdir to subdir: %v", err)
|
||||||
}
|
}
|
||||||
|
ResetCaches() // Reset after chdir
|
||||||
|
|
||||||
root, err := GetMainRepoRoot()
|
root, err := GetMainRepoRoot()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -1012,6 +1020,7 @@ func TestGetMainRepoRoot(t *testing.T) {
|
|||||||
// TestIsWorktree tests the IsWorktree function
|
// TestIsWorktree tests the IsWorktree function
|
||||||
func TestIsWorktree(t *testing.T) {
|
func TestIsWorktree(t *testing.T) {
|
||||||
t.Run("returns false for regular repo", func(t *testing.T) {
|
t.Run("returns false for regular repo", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -1024,6 +1033,7 @@ func TestIsWorktree(t *testing.T) {
|
|||||||
if err := os.Chdir(repoPath); err != nil {
|
if err := os.Chdir(repoPath); err != nil {
|
||||||
t.Fatalf("Failed to chdir to repo: %v", err)
|
t.Fatalf("Failed to chdir to repo: %v", err)
|
||||||
}
|
}
|
||||||
|
ResetCaches() // Reset after chdir
|
||||||
|
|
||||||
if IsWorktree() {
|
if IsWorktree() {
|
||||||
t.Error("IsWorktree() should return false for regular repo")
|
t.Error("IsWorktree() should return false for regular repo")
|
||||||
@@ -1031,6 +1041,7 @@ func TestIsWorktree(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("returns true for worktree", func(t *testing.T) {
|
t.Run("returns true for worktree", func(t *testing.T) {
|
||||||
|
ResetCaches() // Reset caches from previous subtests
|
||||||
repoPath, cleanup := setupTestRepo(t)
|
repoPath, cleanup := setupTestRepo(t)
|
||||||
defer cleanup()
|
defer cleanup()
|
||||||
|
|
||||||
@@ -1051,6 +1062,7 @@ func TestIsWorktree(t *testing.T) {
|
|||||||
t.Fatalf("Failed to chdir to worktree: %v", err)
|
t.Fatalf("Failed to chdir to worktree: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
ResetCaches() // Reset after chdir to worktree
|
||||||
if !IsWorktree() {
|
if !IsWorktree() {
|
||||||
t.Error("IsWorktree() should return true for worktree")
|
t.Error("IsWorktree() should return true for worktree")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user