diff --git a/internal/git/git.go b/internal/git/git.go index 945e4b87..2c128a84 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -82,10 +82,11 @@ func GetCommits(baseSHA, headSHA string, excludePaths []string) ([]Commit, error cmd := exec.Command("git", args...) output, err := cmd.Output() if err != nil { - // No commits in range is not an error - if len(output) == 0 { - return nil, nil - } + // A non-zero exit means git failed (a bad or unknown base SHA, a + // shallow clone missing the base commit, or "not a git repository"). + // A legitimately empty range exits 0 and returns no commits via + // parseCommits below, so any error here is a real failure and must be + // surfaced rather than masked as "no commits." return nil, fmt.Errorf("git log: %w", err) } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index a898c86b..a59cc60f 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -132,6 +132,34 @@ func TestParseCommits(t *testing.T) { } } +// TestGetCommits_UnknownBaseSHAReturnsError proves a git failure (a bad or +// unknown base SHA) is surfaced as an error rather than swallowed as an empty +// commit range. Swallowing it would let the caller silently recompute the same +// version with no bump, cutting a wrong version with no warning. +func TestGetCommits_UnknownBaseSHAReturnsError(t *testing.T) { + newScratchRepo(t) + head := commitFile(t, "a.txt", "one", "first commit") + + if _, err := GetCommits("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", head, nil); err == nil { + t.Fatal("GetCommits() with an unknown base SHA: expected error, got nil") + } +} + +// TestGetCommits_EmptyRangeIsNotAnError proves a legitimately empty range +// (base == head, git exits 0) returns no commits and no error. +func TestGetCommits_EmptyRangeIsNotAnError(t *testing.T) { + newScratchRepo(t) + head := commitFile(t, "a.txt", "one", "first commit") + + commits, err := GetCommits(head, head, nil) + if err != nil { + t.Fatalf("GetCommits() empty range: unexpected error: %v", err) + } + if len(commits) != 0 { + t.Fatalf("GetCommits() empty range: got %d commits, want 0", len(commits)) + } +} + func TestParseLines(t *testing.T) { tests := []struct { name string