From 31bc927386508ea778dcc7a51c057bab7767808b Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Fri, 12 Jun 2026 16:08:47 +0200 Subject: [PATCH] check git metadata writability --- internal/git/git.go | 49 ++++++++++++-- internal/git/git_test.go | 141 ++++++++++++++++++++++++--------------- 2 files changed, 133 insertions(+), 57 deletions(-) diff --git a/internal/git/git.go b/internal/git/git.go index 0697e18..bfab9f9 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -2,19 +2,23 @@ package git import ( "fmt" + "os" "os/exec" + "strings" ) // LookPathFunc is the function used to look up executables in PATH. // It can be overridden in tests. var LookPathFunc = exec.LookPath -// RunGitCommandFunc is the function used to run git commands. +// RunGitOutputFunc is the function used to run git commands and capture output. // It can be overridden in tests. -var RunGitCommandFunc = func(args ...string) error { - return exec.Command("git", args...).Run() +var RunGitOutputFunc = func(args ...string) ([]byte, error) { + return exec.Command("git", args...).CombinedOutput() } +var createTempFileFunc = os.CreateTemp + // CheckAvailable verifies that git is available and the current directory is a git repository. // Returns an error if git is not installed or the current directory is not a git repository. func CheckAvailable() error { @@ -22,9 +26,46 @@ func CheckAvailable() error { return fmt.Errorf("git executable not found: git is required for ddtest to work") } - if err := RunGitCommandFunc("rev-parse", "--git-dir"); err != nil { + gitDir, err := getGitDirectory() + if err != nil { return fmt.Errorf("current directory is not a git repository: git is required for ddtest to work") } + if err := checkWritable(gitDir); err != nil { + return err + } + + return nil +} + +func getGitDirectory() (string, error) { + out, err := RunGitOutputFunc("rev-parse", "--absolute-git-dir") + if err != nil { + return "", err + } + + gitDir := strings.TrimSpace(string(out)) + if gitDir == "" { + return "", fmt.Errorf("git returned an empty git directory") + } + + return gitDir, nil +} + +func checkWritable(path string) error { + file, err := createTempFileFunc(path, ".ddtest-write-check-") + if err != nil { + return fmt.Errorf("git metadata directory is not writable: ddtest needs write access to %s to fetch repository metadata for Test Impact Analysis: %w", path, err) + } + + fileName := file.Name() + if err := file.Close(); err != nil { + _ = os.Remove(fileName) + return fmt.Errorf("git metadata directory write check failed: %s: %w", path, err) + } + if err := os.Remove(fileName); err != nil { + return fmt.Errorf("git metadata directory write check cleanup failed: %s: %w", path, err) + } + return nil } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 87f9424..1d3538d 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -2,19 +2,45 @@ package git import ( "errors" + "fmt" + "os" "os/exec" "strings" "testing" ) -func TestCheckAvailable_Success(t *testing.T) { - // Save original functions +func resetGitHooks(t *testing.T) { + t.Helper() + origLookPath := LookPathFunc - origRunGit := RunGitCommandFunc - defer func() { + origRunGitOutput := RunGitOutputFunc + origCreateTempFile := createTempFileFunc + t.Cleanup(func() { LookPathFunc = origLookPath - RunGitCommandFunc = origRunGit - }() + RunGitOutputFunc = origRunGitOutput + createTempFileFunc = origCreateTempFile + }) +} + +func mockGitDirectory(t *testing.T) string { + t.Helper() + + gitDir := t.TempDir() + RunGitOutputFunc = func(args ...string) ([]byte, error) { + switch strings.Join(args, " ") { + case "rev-parse --absolute-git-dir": + return []byte(gitDir + "\n"), nil + default: + return nil, fmt.Errorf("unexpected git command: %v", args) + } + } + + return gitDir +} + +func TestCheckAvailable_Success(t *testing.T) { + resetGitHooks(t) + mockGitDirectory(t) // Mock both functions to succeed LookPathFunc = func(file string) (string, error) { @@ -23,9 +49,6 @@ func TestCheckAvailable_Success(t *testing.T) { } return "", errors.New("not found") } - RunGitCommandFunc = func(args ...string) error { - return nil - } err := CheckAvailable() if err != nil { @@ -34,21 +57,12 @@ func TestCheckAvailable_Success(t *testing.T) { } func TestCheckAvailable_GitNotInstalled(t *testing.T) { - // Save original functions - origLookPath := LookPathFunc - origRunGit := RunGitCommandFunc - defer func() { - LookPathFunc = origLookPath - RunGitCommandFunc = origRunGit - }() + resetGitHooks(t) // Mock LookPath to fail LookPathFunc = func(file string) (string, error) { return "", errors.New("executable file not found in $PATH") } - RunGitCommandFunc = func(args ...string) error { - return nil - } err := CheckAvailable() if err == nil { @@ -63,13 +77,7 @@ func TestCheckAvailable_GitNotInstalled(t *testing.T) { } func TestCheckAvailable_NotAGitRepo(t *testing.T) { - // Save original functions - origLookPath := LookPathFunc - origRunGit := RunGitCommandFunc - defer func() { - LookPathFunc = origLookPath - RunGitCommandFunc = origRunGit - }() + resetGitHooks(t) // Mock LookPath to succeed, but git command to fail LookPathFunc = func(file string) (string, error) { @@ -78,8 +86,8 @@ func TestCheckAvailable_NotAGitRepo(t *testing.T) { } return "", errors.New("not found") } - RunGitCommandFunc = func(args ...string) error { - return errors.New("fatal: not a git repository") + RunGitOutputFunc = func(args ...string) ([]byte, error) { + return nil, errors.New("fatal: not a git repository") } err := CheckAvailable() @@ -95,37 +103,65 @@ func TestCheckAvailable_NotAGitRepo(t *testing.T) { } func TestCheckAvailable_CorrectGitCommand(t *testing.T) { - // Save original functions - origLookPath := LookPathFunc - origRunGit := RunGitCommandFunc - defer func() { - LookPathFunc = origLookPath - RunGitCommandFunc = origRunGit - }() + resetGitHooks(t) - var capturedArgs []string + gitDir := t.TempDir() + var capturedOutputArgs []string LookPathFunc = func(file string) (string, error) { return "/usr/bin/git", nil } - RunGitCommandFunc = func(args ...string) error { - capturedArgs = args - return nil + RunGitOutputFunc = func(args ...string) ([]byte, error) { + capturedOutputArgs = append([]string(nil), args...) + switch strings.Join(args, " ") { + case "rev-parse --absolute-git-dir": + return []byte(gitDir + "\n"), nil + default: + return nil, fmt.Errorf("unexpected git command: %v", args) + } } - _ = CheckAvailable() + if err := CheckAvailable(); err != nil { + t.Fatalf("expected no error, got %v", err) + } - expectedArgs := []string{"rev-parse", "--git-dir"} - if len(capturedArgs) != len(expectedArgs) { - t.Errorf("expected args %v, got %v", expectedArgs, capturedArgs) + expectedArgs := []string{"rev-parse", "--absolute-git-dir"} + if len(capturedOutputArgs) != len(expectedArgs) { + t.Fatalf("expected args %v, got %v", expectedArgs, capturedOutputArgs) } for i, arg := range expectedArgs { - if capturedArgs[i] != arg { - t.Errorf("expected arg[%d] = %q, got %q", i, arg, capturedArgs[i]) + if capturedOutputArgs[i] != arg { + t.Errorf("expected arg[%d] = %q, got %q", i, arg, capturedOutputArgs[i]) } } } +func TestCheckAvailable_GitDirectoryNotWritable(t *testing.T) { + resetGitHooks(t) + gitDir := mockGitDirectory(t) + + LookPathFunc = func(file string) (string, error) { + return "/usr/bin/git", nil + } + createTempFileFunc = func(dir, pattern string) (*os.File, error) { + return nil, os.ErrPermission + } + + err := CheckAvailable() + if err == nil { + t.Fatal("expected error when git directory is not writable") + } + if !strings.Contains(err.Error(), "git metadata directory is not writable") { + t.Errorf("expected git metadata writability error, got: %v", err) + } + if !strings.Contains(err.Error(), gitDir) { + t.Errorf("expected git directory path %q in error message, got: %v", gitDir, err) + } + if !strings.Contains(err.Error(), "ddtest needs write access") { + t.Errorf("expected actionable write access message, got: %v", err) + } +} + func TestCheckAvailable_Integration(t *testing.T) { // This test uses real git commands to verify the function works in a real environment // Skip if git is not available @@ -134,22 +170,21 @@ func TestCheckAvailable_Integration(t *testing.T) { } // Save original functions to ensure we use real ones - origLookPath := LookPathFunc - origRunGit := RunGitCommandFunc - defer func() { - LookPathFunc = origLookPath - RunGitCommandFunc = origRunGit - }() + resetGitHooks(t) // Reset to real implementations LookPathFunc = exec.LookPath - RunGitCommandFunc = func(args ...string) error { - return exec.Command("git", args...).Run() + RunGitOutputFunc = func(args ...string) ([]byte, error) { + return exec.Command("git", args...).CombinedOutput() } + createTempFileFunc = os.CreateTemp // Since tests run in the project directory which is a git repo, this should succeed err := CheckAvailable() if err != nil { + if strings.Contains(err.Error(), "git metadata directory is not writable") { + t.Skipf("git metadata directory is not writable in this environment: %v", err) + } t.Errorf("expected no error in git repository, got %v", err) } }