From 6a13b5a64d0300bb31f72d94e8d50a9582080e2f Mon Sep 17 00:00:00 2001 From: Lukasz Mierzwa Date: Fri, 21 Jun 2024 13:37:04 +0100 Subject: [PATCH] Refactor git history parsing When running pint ci on a branch with merge commits these commits would make it look like there were more changes than in the actual diff. --- cmd/pint/tests/0028_ci_git_error.txt | 4 +- cmd/pint/tests/0065_ci_include.txt | 2 - .../tests/0134_ci_base_branch_flag_path.txt | 4 +- .../tests/0135_ci_base_branch_config_path.txt | 4 +- cmd/pint/tests/0139_ci_exclude.txt | 2 - cmd/pint/tests/0140_ci_include_exclude.txt | 2 - docs/changelog.md | 1 + internal/discovery/git_branch.go | 24 +++--- internal/discovery/git_branch_test.go | 40 ++++----- internal/git/changes.go | 4 +- internal/git/changes_test.go | 86 +++++++------------ internal/git/git.go | 36 -------- internal/git/git_test.go | 79 ----------------- 13 files changed, 71 insertions(+), 217 deletions(-) diff --git a/cmd/pint/tests/0028_ci_git_error.txt b/cmd/pint/tests/0028_ci_git_error.txt index 2249560b..609d7576 100644 --- a/cmd/pint/tests/0028_ci_git_error.txt +++ b/cmd/pint/tests/0028_ci_git_error.txt @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=* level=DEBUG msg="File parsed" path=.pint.hcl rules=0 level=DEBUG msg="File parsed" path=rules.yml rules=2 level=DEBUG msg="Glob finder completed" count=2 -level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","notmain..HEAD"] -level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'notmain..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" +level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","notmain..HEAD"] +level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'notmain..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" -- src/v1.yml -- - record: rule1 expr: sum(foo) by(job) diff --git a/cmd/pint/tests/0065_ci_include.txt b/cmd/pint/tests/0065_ci_include.txt index 00ee8f02..37ca59cc 100644 --- a/cmd/pint/tests/0065_ci_include.txt +++ b/cmd/pint/tests/0065_ci_include.txt @@ -23,8 +23,6 @@ exec git commit -am 'v2' pint.ok -l debug --no-color ci ! stdout . stderr 'level=DEBUG msg="Got branch information" base=main current=v2' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*' stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml' diff --git a/cmd/pint/tests/0134_ci_base_branch_flag_path.txt b/cmd/pint/tests/0134_ci_base_branch_flag_path.txt index 72274821..5b2fc91e 100644 --- a/cmd/pint/tests/0134_ci_base_branch_flag_path.txt +++ b/cmd/pint/tests/0134_ci_base_branch_flag_path.txt @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=* level=DEBUG msg="File parsed" path=.pint.hcl rules=0 level=DEBUG msg="File parsed" path=rules.yml rules=1 level=DEBUG msg="Glob finder completed" count=1 -level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","origin/main..HEAD"] -level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" +level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","origin/main..HEAD"] +level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" -- src/v1.yml -- - record: rule1 expr: sum(foo) by(job) diff --git a/cmd/pint/tests/0135_ci_base_branch_config_path.txt b/cmd/pint/tests/0135_ci_base_branch_config_path.txt index a22102f5..bcd99329 100644 --- a/cmd/pint/tests/0135_ci_base_branch_config_path.txt +++ b/cmd/pint/tests/0135_ci_base_branch_config_path.txt @@ -28,8 +28,8 @@ level=DEBUG msg="Excluding git directory from glob results" path=.git glob=* level=DEBUG msg="File parsed" path=.pint.hcl rules=0 level=DEBUG msg="File parsed" path=rules.yml rules=1 level=DEBUG msg="Glob finder completed" count=1 -level=DEBUG msg="Running git command" args=["log","--format=%H","--no-abbrev-commit","--reverse","origin/main..HEAD"] -level=ERROR msg="Fatal error" err="failed to get the list of commits to scan: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" +level=DEBUG msg="Running git command" args=["log","--reverse","--no-merges","--first-parent","--format=%H","--name-status","origin/main..HEAD"] +level=ERROR msg="Fatal error" err="failed to get the list of modified files from git: fatal: ambiguous argument 'origin/main..HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n" -- src/v1.yml -- - record: rule1 expr: sum(foo) by(job) diff --git a/cmd/pint/tests/0139_ci_exclude.txt b/cmd/pint/tests/0139_ci_exclude.txt index 1089bdec..013672ff 100644 --- a/cmd/pint/tests/0139_ci_exclude.txt +++ b/cmd/pint/tests/0139_ci_exclude.txt @@ -23,8 +23,6 @@ exec git commit -am 'v2' pint.ok -l debug --no-color ci ! stdout . stderr 'level=DEBUG msg="Got branch information" base=main current=v2' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*' stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml' diff --git a/cmd/pint/tests/0140_ci_include_exclude.txt b/cmd/pint/tests/0140_ci_include_exclude.txt index ee96236c..14239204 100644 --- a/cmd/pint/tests/0140_ci_include_exclude.txt +++ b/cmd/pint/tests/0140_ci_include_exclude.txt @@ -23,8 +23,6 @@ exec git commit -am 'v2' pint.ok -l debug --no-color ci ! stdout . stderr 'level=DEBUG msg="Got branch information" base=main current=v2' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' -stderr 'level=DEBUG msg="Found commit to scan" commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=a.yml commit=.*' stderr 'level=DEBUG msg="Git file change" change=A path=b.yml commit=.*' stderr 'level=DEBUG msg="Skipping file due to include/exclude rules" path=a.yml' diff --git a/docs/changelog.md b/docs/changelog.md index 13667b62..7b0a8cd8 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -5,6 +5,7 @@ ### Fixed - Don't suggest using `humanize` when alert template is already using printf on format the `$value`. +- Fixed git history parsing when running `pint ci` on a branch that include merge commits. ## v0.60.0 diff --git a/internal/discovery/git_branch.go b/internal/discovery/git_branch.go index 32551ff3..9c4822e8 100644 --- a/internal/discovery/git_branch.go +++ b/internal/discovery/git_branch.go @@ -39,19 +39,13 @@ func (f GitBranchFinder) Find(allEntries []Entry) (entries []Entry, err error) { allEntries[i].State = Excluded } - cr, err := git.CommitRange(f.gitCmd, f.baseBranch) + changes, err := git.Changes(f.gitCmd, f.baseBranch, f.filter) if err != nil { - return nil, fmt.Errorf("failed to get the list of commits to scan: %w", err) + return nil, err } - slog.Debug("Got commit range from git", slog.String("from", cr.From), slog.String("to", cr.To)) - if len(cr.Commits) > f.maxCommits { - return nil, fmt.Errorf("number of commits to check (%d) is higher than maxCommits (%d), exiting", len(cr.Commits), f.maxCommits) - } - - changes, err := git.Changes(f.gitCmd, cr, f.filter) - if err != nil { - return nil, err + if totalCommits := countCommits(changes); totalCommits > f.maxCommits { + return nil, fmt.Errorf("number of commits to check (%d) is higher than maxCommits (%d), exiting", totalCommits, f.maxCommits) } shouldSkip, err := f.shouldSkipAllChecks(changes) @@ -321,3 +315,13 @@ func findRulesByName(entries []Entry, name string, typ parser.RuleType) (nomatch } return nomatch, match } + +func countCommits(changes []*git.FileChange) int { + commits := map[string]struct{}{} + for _, change := range changes { + for _, commit := range change.Commits { + commits[commit] = struct{}{} + } + } + return len(commits) +} diff --git a/internal/discovery/git_branch_test.go b/internal/discovery/git_branch_test.go index 22bcfcc6..5dfcbacc 100644 --- a/internal/discovery/git_branch_test.go +++ b/internal/discovery/git_branch_test.go @@ -80,7 +80,7 @@ func TestGitBranchFinder(t *testing.T) { 50, ), entries: nil, - err: "failed to get the list of commits to scan: mock git error: [log --format=%H --no-abbrev-commit --reverse main..HEAD]", + err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD]", }, { title: "git list PR commits error - master", @@ -94,24 +94,22 @@ func TestGitBranchFinder(t *testing.T) { 50, ), entries: nil, - err: "failed to get the list of commits to scan: mock git error: [log --format=%H --no-abbrev-commit --reverse master..HEAD]", + err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status master..HEAD]", }, { title: "too many commits", - setup: func(_ *testing.T) {}, - finder: discovery.NewGitBranchFinder( - func(args ...string) ([]byte, error) { - switch strings.Join(args, " ") { - case "log --format=%H --no-abbrev-commit --reverse main..HEAD": - return []byte("c1\nc2\nc3\nc4\n"), nil - default: - return nil, fmt.Errorf("mock git error: %v", args) - } - }, - git.NewPathFilter(includeAll, nil, nil), - "main", - 3, - ), + setup: func(t *testing.T) { + commitFile(t, "rules.yml", "# v1\n", "v1") + + _, err := git.RunGit("checkout", "-b", "v2") + require.NoError(t, err, "git checkout v2") + + commitFile(t, "rules.yml", "# v2-1\n", "v2-1") + commitFile(t, "rules.yml", "# v2-2\n", "v2-2") + commitFile(t, "rules.yml", "# v2-3\n", "v2-3") + commitFile(t, "rules.yml", "# v2-4\n", "v2-4") + }, + finder: discovery.NewGitBranchFinder(git.RunGit, git.NewPathFilter(includeAll, nil, nil), "main", 3), entries: nil, err: "number of commits to check (4) is higher than maxCommits (3), exiting", }, @@ -132,7 +130,7 @@ func TestGitBranchFinder(t *testing.T) { 4, ), entries: nil, - err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4]", + err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD]", }, { title: "git get commit message error", @@ -140,9 +138,7 @@ func TestGitBranchFinder(t *testing.T) { finder: discovery.NewGitBranchFinder( func(args ...string) ([]byte, error) { switch strings.Join(args, " ") { - case "log --format=%H --no-abbrev-commit --reverse main..HEAD": - return []byte("c1\nc2\nc3\nc4\n"), nil - case "log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4": + case "log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD": return []byte("c1\nA\trules.yml\n"), nil default: return nil, fmt.Errorf("mock git error: %v", args) @@ -161,9 +157,7 @@ func TestGitBranchFinder(t *testing.T) { finder: discovery.NewGitBranchFinder( func(args ...string) ([]byte, error) { switch strings.Join(args, " ") { - case "log --format=%H --no-abbrev-commit --reverse main..HEAD": - return []byte("c1\nc2\nc3\nc4\n"), nil - case "log --reverse --no-merges --first-parent --format=%H --name-status c1^..c4": + case "log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD": return []byte("c1\nA\trules.yml\n"), nil case "ls-tree c1^ rules.yml": return []byte("100644 blob c0\trules.yml"), nil diff --git a/internal/git/changes.go b/internal/git/changes.go index 54c05751..c328ff65 100644 --- a/internal/git/changes.go +++ b/internal/git/changes.go @@ -68,8 +68,8 @@ type FileChange struct { Status FileStatus } -func Changes(cmd CommandRunner, cr CommitRangeResults, filter PathFilter) ([]*FileChange, error) { - out, err := cmd("log", "--reverse", "--no-merges", "--first-parent", "--format=%H", "--name-status", cr.String()) +func Changes(cmd CommandRunner, baseBranch string, filter PathFilter) ([]*FileChange, error) { + out, err := cmd("log", "--reverse", "--no-merges", "--first-parent", "--format=%H", "--name-status", fmt.Sprintf("%s..HEAD", baseBranch)) if err != nil { return nil, fmt.Errorf("failed to get the list of modified files from git: %w", err) } diff --git a/internal/git/changes_test.go b/internal/git/changes_test.go index 43339b58..b6ff4a3b 100644 --- a/internal/git/changes_test.go +++ b/internal/git/changes_test.go @@ -42,7 +42,7 @@ func gitCommit(t *testing.T, message string) { func TestChanges(t *testing.T) { type testCaseT struct { - setup func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) + setup func(t *testing.T) git.CommandRunner title string err string changes []*git.FileChange @@ -51,19 +51,18 @@ func TestChanges(t *testing.T) { testCases := []testCaseT{ { title: "git log error", - setup: func(_ *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(_ *testing.T) git.CommandRunner { cmd := func(args ...string) ([]byte, error) { return nil, fmt.Errorf("mock git error: %v", args) } - cr := git.CommitRangeResults{From: "a", To: "b"} - return cmd, cr + return cmd }, changes: nil, - err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status a^..b]", + err: "failed to get the list of modified files from git: mock git error: [log --reverse --no-merges --first-parent --format=%H --name-status main..HEAD]", }, { title: "chmod", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -74,9 +73,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "index.txt") gitCommit(t, "chmod") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -102,7 +99,7 @@ func TestChanges(t *testing.T) { }, { title: "dir -> file", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.Mkdir("index.txt", 0o755)) require.NoError(t, os.WriteFile("index.txt/.keep", []byte("keep"), 0o644)) @@ -115,9 +112,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "index.txt") gitCommit(t, "chmod") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -161,7 +156,7 @@ func TestChanges(t *testing.T) { }, { title: "delete and re-add", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -175,9 +170,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "index.txt") gitCommit(t, "add") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -203,7 +196,7 @@ func TestChanges(t *testing.T) { }, { title: "file -> symlink", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo\n1\n"), 0o644)) mustRun(t, "add", "index.txt") @@ -217,9 +210,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "second file.txt") gitCommit(t, "symlink") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -246,7 +237,7 @@ func TestChanges(t *testing.T) { }, { title: "rename partial", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("1\n2\n3\n4\n5\n6\n7\n8\n9\n"), 0o644)) mustRun(t, "add", "index.txt") @@ -258,9 +249,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "second.txt") gitCommit(t, "mv") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -286,7 +275,7 @@ func TestChanges(t *testing.T) { }, { title: "rename 100% and edit", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("1\n2\n3\n4\n5\n6\n7\n8\n9\n"), 0o644)) mustRun(t, "add", "index.txt") @@ -299,9 +288,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "second.txt") gitCommit(t, "edit") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -327,7 +314,7 @@ func TestChanges(t *testing.T) { }, { title: "add file, add another", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -340,9 +327,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "third.txt") gitCommit(t, "add two more") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -384,7 +369,7 @@ func TestChanges(t *testing.T) { }, { title: "delete file", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -397,9 +382,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "second.txt") gitCommit(t, "rm second") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -424,7 +407,7 @@ func TestChanges(t *testing.T) { }, { title: "delete symlink", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -437,9 +420,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "second.txt") gitCommit(t, "rm second") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -465,7 +446,7 @@ func TestChanges(t *testing.T) { }, { title: "delete directory with symlinks", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo"), 0o644)) mustRun(t, "add", "index.txt") @@ -480,9 +461,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "dir") gitCommit(t, "rm dir") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -526,7 +505,7 @@ func TestChanges(t *testing.T) { }, { title: "symlink target changed", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("index.txt", []byte("foo\n1\n"), 0o644)) mustRun(t, "add", "index.txt") @@ -544,9 +523,7 @@ func TestChanges(t *testing.T) { mustRun(t, "add", "dir") gitCommit(t, "symlink change") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -574,7 +551,7 @@ func TestChanges(t *testing.T) { }, { title: "rule modified then file renamed", - setup: func(t *testing.T) (git.CommandRunner, git.CommitRangeResults) { + setup: func(t *testing.T) git.CommandRunner { mustRun(t, "init", "--initial-branch=main", ".") require.NoError(t, os.WriteFile("main.txt", []byte("l1\nl2\nl3\n"), 0o644)) mustRun(t, "add", "main.txt") @@ -588,9 +565,7 @@ func TestChanges(t *testing.T) { mustRun(t, "mv", "main.txt", "pr.txt") gitCommit(t, "rename") - cr, err := git.CommitRange(debugGitRun(t), "main") - require.NoError(t, err) - return debugGitRun(t), cr + return debugGitRun(t) }, changes: []*git.FileChange{ { @@ -624,13 +599,14 @@ func TestChanges(t *testing.T) { err := os.Chdir(dir) require.NoError(t, err, "chdir") - cmd, cr := tc.setup(t) - changes, err := git.Changes(cmd, cr, git.NewPathFilter(nil, nil, nil)) + cmd := tc.setup(t) + changes, err := git.Changes(cmd, "main", git.NewPathFilter(nil, nil, nil)) if tc.err != "" { require.EqualError(t, err, tc.err) require.Nil(t, changes) } else { require.NoError(t, err) + require.Len(t, changes, len(tc.changes)) for i := range tc.changes { require.Len(t, changes[i].Commits, len(tc.changes[i].Commits), "changes[%d].Commits", i) require.Equal(t, tc.changes[i].Path, changes[i].Path, "changes[%d].Path", i) diff --git a/internal/git/git.go b/internal/git/git.go index ec2a675b..dd64a101 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -98,42 +98,6 @@ func HeadCommit(cmd CommandRunner) (string, error) { return strings.Trim(string(commit), "\n"), nil } -type CommitRangeResults struct { - From string - To string - Commits []string -} - -func (gcr CommitRangeResults) String() string { - return fmt.Sprintf("%s^..%s", gcr.From, gcr.To) -} - -func CommitRange(cmd CommandRunner, baseBranch string) (CommitRangeResults, error) { - cr := CommitRangeResults{Commits: []string{}} - - out, err := cmd("log", "--format=%H", "--no-abbrev-commit", "--reverse", fmt.Sprintf("%s..HEAD", baseBranch)) - if err != nil { - return cr, err - } - - for _, line := range strings.Split(strings.TrimSuffix(string(out), "\n"), "\n") { - if line != "" { - cr.Commits = append(cr.Commits, line) - if cr.From == "" { - cr.From = line - } - cr.To = line - slog.Debug("Found commit to scan", slog.String("commit", line)) - } - } - - if len(cr.Commits) == 0 { - return cr, fmt.Errorf("empty commit range") - } - - return cr, nil -} - func CurrentBranch(cmd CommandRunner) (string, error) { commit, err := cmd("rev-parse", "--abbrev-ref", "HEAD") if err != nil { diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 0bd19b14..7b330420 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -135,85 +135,6 @@ func TestGitBlame(t *testing.T) { } } -func TestCommitRange(t *testing.T) { - type testCaseT struct { - mock git.CommandRunner - output git.CommitRangeResults - shouldError bool - } - - testCases := []testCaseT{ - { - mock: func(_ ...string) ([]byte, error) { - return nil, fmt.Errorf("mock error") - }, - output: git.CommitRangeResults{Commits: []string{}}, - shouldError: true, - }, - { - mock: func(_ ...string) ([]byte, error) { - return []byte(""), nil - }, - output: git.CommitRangeResults{Commits: []string{}}, - shouldError: true, - }, - { - mock: func(_ ...string) ([]byte, error) { - return []byte("commit1\n"), nil - }, - output: git.CommitRangeResults{ - Commits: []string{"commit1"}, - From: "commit1", - To: "commit1", - }, - }, - { - mock: func(_ ...string) ([]byte, error) { - return []byte("commit1\ncommit2\ncommit3\n"), nil - }, - output: git.CommitRangeResults{ - Commits: []string{"commit1", "commit2", "commit3"}, - From: "commit1", - To: "commit3", - }, - }, - { - mock: func(_ ...string) ([]byte, error) { - return []byte("commit2\ncommit1"), nil - }, - output: git.CommitRangeResults{ - Commits: []string{"commit2", "commit1"}, - From: "commit2", - To: "commit1", - }, - }, - { - mock: func(_ ...string) ([]byte, error) { - return []byte("commit2\ncommit1\n"), nil - }, - output: git.CommitRangeResults{ - Commits: []string{"commit2", "commit1"}, - From: "commit2", - To: "commit1", - }, - }, - } - - for i, tc := range testCases { - t.Run(strconv.Itoa(i), func(t *testing.T) { - output, err := git.CommitRange(tc.mock, "main") - - hadError := (err != nil) - if hadError != tc.shouldError { - t.Errorf("git.CommitRange() returned err=%v, expected=%v", err, tc.shouldError) - return - } - - require.Equal(t, tc.output, output, "git.CommitRange() returned wrong output") - }) - } -} - func TestCurrentBranch(t *testing.T) { type testCaseT struct { mock git.CommandRunner