Files
review-bot/vcs/util_test.go
T
claw 90959da830
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 32s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 57s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m40s
feat(vcs): add util.go with GetAllFilesInPath and BuildLineToPositionMap (#84)
Add vcs/util.go containing two utility functions that were specified
in issue #78 but omitted from PR #83:

- GetAllFilesInPath: recursively fetches all file contents under a path
  using the vcs.FileReader interface. Returns map[string]string of
  path -> content.

- BuildLineToPositionMap: parses a unified diff and returns per-file
  mapping of new-file line numbers to GitHub diff-position convention.
  Position is 1-indexed from @@ hunk headers. Deletion lines are not
  mapped (no new-file line number). Position counter continues across
  hunks within the same file but resets for each new file.

Unit tests cover:
- GetAllFilesInPath: empty dir, flat dir, nested dirs, mixed, errors
- BuildLineToPositionMap: single hunk, multi-hunk, deletions not mapped,
  multiple files, empty diff, no-newline marker, single-line hunk,
  deleted file
2026-05-12 12:31:10 -07:00

457 lines
11 KiB
Go

package vcs_test
import (
"context"
"fmt"
"testing"
"gitea.weiker.me/rodin/review-bot/vcs"
)
// mockFileReader implements vcs.FileReader for testing.
type mockFileReader struct {
contents map[string]string // path -> content
dirs map[string][]vcs.ContentEntry // path -> entries
}
func (m *mockFileReader) GetFileContent(_ context.Context, _, _, path, _ string) (string, error) {
content, ok := m.contents[path]
if !ok {
return "", fmt.Errorf("file not found: %s", path)
}
return content, nil
}
func (m *mockFileReader) ListContents(_ context.Context, _, _, path string) ([]vcs.ContentEntry, error) {
entries, ok := m.dirs[path]
if !ok {
return nil, fmt.Errorf("directory not found: %s", path)
}
return entries, nil
}
// --- GetAllFilesInPath tests ---
func TestGetAllFilesInPath_EmptyDir(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{
"src": {},
},
}
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(result) != 0 {
t.Errorf("expected empty map, got %d entries", len(result))
}
}
func TestGetAllFilesInPath_FlatDir(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{
"src": {
{Name: "main.go", Path: "src/main.go", Type: "file"},
{Name: "util.go", Path: "src/util.go", Type: "file"},
},
},
contents: map[string]string{
"src/main.go": "package main",
"src/util.go": "package main\n\nfunc helper() {}",
},
}
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(result) != 2 {
t.Fatalf("expected 2 files, got %d", len(result))
}
if result["src/main.go"] != "package main" {
t.Errorf("unexpected content for main.go: %q", result["src/main.go"])
}
if result["src/util.go"] != "package main\n\nfunc helper() {}" {
t.Errorf("unexpected content for util.go: %q", result["src/util.go"])
}
}
func TestGetAllFilesInPath_NestedDirs(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{
"src": {
{Name: "main.go", Path: "src/main.go", Type: "file"},
{Name: "pkg", Path: "src/pkg", Type: "dir"},
},
"src/pkg": {
{Name: "lib.go", Path: "src/pkg/lib.go", Type: "file"},
{Name: "internal", Path: "src/pkg/internal", Type: "dir"},
},
"src/pkg/internal": {
{Name: "helper.go", Path: "src/pkg/internal/helper.go", Type: "file"},
},
},
contents: map[string]string{
"src/main.go": "package main",
"src/pkg/lib.go": "package pkg",
"src/pkg/internal/helper.go": "package internal",
},
}
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(result) != 3 {
t.Fatalf("expected 3 files, got %d", len(result))
}
if result["src/main.go"] != "package main" {
t.Errorf("unexpected content for main.go")
}
if result["src/pkg/lib.go"] != "package pkg" {
t.Errorf("unexpected content for lib.go")
}
if result["src/pkg/internal/helper.go"] != "package internal" {
t.Errorf("unexpected content for helper.go")
}
}
func TestGetAllFilesInPath_Mixed(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{
"root": {
{Name: "README.md", Path: "root/README.md", Type: "file"},
{Name: "empty", Path: "root/empty", Type: "dir"},
{Name: "docs", Path: "root/docs", Type: "dir"},
},
"root/empty": {},
"root/docs": {
{Name: "guide.md", Path: "root/docs/guide.md", Type: "file"},
},
},
contents: map[string]string{
"root/README.md": "# Hello",
"root/docs/guide.md": "## Guide",
},
}
result, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "root")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(result) != 2 {
t.Fatalf("expected 2 files, got %d", len(result))
}
if result["root/README.md"] != "# Hello" {
t.Errorf("unexpected content for README.md")
}
if result["root/docs/guide.md"] != "## Guide" {
t.Errorf("unexpected content for guide.md")
}
}
func TestGetAllFilesInPath_ListContentsError(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{},
}
_, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "missing")
if err == nil {
t.Fatal("expected error for missing directory")
}
}
func TestGetAllFilesInPath_GetFileContentError(t *testing.T) {
mock := &mockFileReader{
dirs: map[string][]vcs.ContentEntry{
"src": {
{Name: "bad.go", Path: "src/bad.go", Type: "file"},
},
},
contents: map[string]string{},
}
_, err := vcs.GetAllFilesInPath(context.Background(), mock, "owner", "repo", "src")
if err == nil {
t.Fatal("expected error when file content fetch fails")
}
}
// --- BuildLineToPositionMap tests ---
func TestBuildLineToPositionMap_SingleHunk(t *testing.T) {
diff := `diff --git a/main.go b/main.go
index abc..def 100644
--- a/main.go
+++ b/main.go
@@ -5,7 +5,8 @@ package main
import "fmt"
func main() {
- fmt.Println("old")
+ fmt.Println("new")
+ fmt.Println("added")
return
}
`
result := vcs.BuildLineToPositionMap(diff)
fileMap, ok := result["main.go"]
if !ok {
t.Fatal("expected main.go in result")
}
// @@ line is position 1
// " import \"fmt\"" -> pos 2, newLine 5
// " " -> pos 3, newLine 6
// " func main() {" -> pos 4, newLine 7
// "-\tfmt.Println..." -> pos 5, no new line
// "+\tfmt.Println..." -> pos 6, newLine 8
// "+\tfmt.Println..." -> pos 7, newLine 9
// " \treturn" -> pos 8, newLine 10
// " }" -> pos 9, newLine 11
expected := map[int]int{
5: 2,
6: 3,
7: 4,
8: 6,
9: 7,
10: 8,
11: 9,
}
for line, wantPos := range expected {
gotPos, exists := fileMap[line]
if !exists {
t.Errorf("line %d: expected position %d, but line not in map", line, wantPos)
continue
}
if gotPos != wantPos {
t.Errorf("line %d: expected position %d, got %d", line, wantPos, gotPos)
}
}
if len(fileMap) != len(expected) {
t.Errorf("expected %d entries, got %d", len(expected), len(fileMap))
}
}
func TestBuildLineToPositionMap_MultiHunk(t *testing.T) {
diff := `diff --git a/main.go b/main.go
--- a/main.go
+++ b/main.go
@@ -1,3 +1,4 @@
package main
+import "fmt"
func main() {
@@ -10,3 +11,4 @@ func main() {
return
+ // extra
}
`
result := vcs.BuildLineToPositionMap(diff)
fileMap, ok := result["main.go"]
if !ok {
t.Fatal("expected main.go in result")
}
// First hunk:
// @@ line -> pos 1
// " package main" -> pos 2, newLine 1
// " " -> pos 3, newLine 2
// "+import..." -> pos 4, newLine 3
// " func main.." -> pos 5, newLine 4
//
// Second hunk (position continues!):
// @@ line -> pos 6
// " \treturn" -> pos 7, newLine 11
// "+\t// extra" -> pos 8, newLine 12
// " }" -> pos 9, newLine 13
expected := map[int]int{
1: 2,
2: 3,
3: 4,
4: 5,
11: 7,
12: 8,
13: 9,
}
for line, wantPos := range expected {
gotPos, exists := fileMap[line]
if !exists {
t.Errorf("line %d: expected position %d, but line not in map", line, wantPos)
continue
}
if gotPos != wantPos {
t.Errorf("line %d: expected position %d, got %d", line, wantPos, gotPos)
}
}
}
func TestBuildLineToPositionMap_DeletionLinesNotInMap(t *testing.T) {
diff := `diff --git a/old.go b/old.go
--- a/old.go
+++ b/old.go
@@ -1,3 +1,1 @@
-line one
-line two
remaining
`
result := vcs.BuildLineToPositionMap(diff)
fileMap, ok := result["old.go"]
if !ok {
t.Fatal("expected old.go in result")
}
// @@ line -> pos 1
// "-line one" -> pos 2, no new line
// "-line two" -> pos 3, no new line
// " remaining" -> pos 4, newLine 1
if pos, ok := fileMap[1]; !ok || pos != 4 {
t.Errorf("line 1: expected position 4, got %d (exists=%v)", pos, ok)
}
if len(fileMap) != 1 {
t.Errorf("expected 1 entry (only 'remaining'), got %d", len(fileMap))
}
}
func TestBuildLineToPositionMap_MultipleFiles(t *testing.T) {
diff := `diff --git a/a.go b/a.go
--- a/a.go
+++ b/a.go
@@ -1,2 +1,3 @@
package a
+// added
diff --git a/b.go b/b.go
new file mode 100644
--- /dev/null
+++ b/b.go
@@ -0,0 +1,3 @@
+package b
+
+func B() {}
`
result := vcs.BuildLineToPositionMap(diff)
// File a.go
aMap, ok := result["a.go"]
if !ok {
t.Fatal("expected a.go in result")
}
// @@ line -> pos 1
// " package a" -> pos 2, newLine 1
// "+// added" -> pos 3, newLine 2
// " " -> pos 4, newLine 3
if aMap[1] != 2 {
t.Errorf("a.go line 1: expected pos 2, got %d", aMap[1])
}
if aMap[2] != 3 {
t.Errorf("a.go line 2: expected pos 3, got %d", aMap[2])
}
if aMap[3] != 4 {
t.Errorf("a.go line 3: expected pos 4, got %d", aMap[3])
}
// File b.go — position resets for new file
bMap, ok := result["b.go"]
if !ok {
t.Fatal("expected b.go in result")
}
// @@ line -> pos 1
// "+package b" -> pos 2, newLine 1
// "+" -> pos 3, newLine 2
// "+func B() {}" -> pos 4, newLine 3
if bMap[1] != 2 {
t.Errorf("b.go line 1: expected pos 2, got %d", bMap[1])
}
if bMap[2] != 3 {
t.Errorf("b.go line 2: expected pos 3, got %d", bMap[2])
}
if bMap[3] != 4 {
t.Errorf("b.go line 3: expected pos 4, got %d", bMap[3])
}
}
func TestBuildLineToPositionMap_EmptyDiff(t *testing.T) {
result := vcs.BuildLineToPositionMap("")
if len(result) != 0 {
t.Errorf("expected empty map for empty diff, got %d entries", len(result))
}
}
func TestBuildLineToPositionMap_NoNewlineMarker(t *testing.T) {
diff := `diff --git a/a.go b/a.go
--- a/a.go
+++ b/a.go
@@ -1,2 +1,2 @@
-old
+new
\ No newline at end of file
`
result := vcs.BuildLineToPositionMap(diff)
fileMap := result["a.go"]
// @@ line -> pos 1
// "-old" -> pos 2
// "+new" -> pos 3, newLine 1
// "\ No.." -> not counted
if fileMap[1] != 3 {
t.Errorf("line 1: expected position 3, got %d", fileMap[1])
}
if len(fileMap) != 1 {
t.Errorf("expected 1 entry, got %d", len(fileMap))
}
}
func TestBuildLineToPositionMap_SingleLineHunk(t *testing.T) {
diff := `diff --git a/single.go b/single.go
--- a/single.go
+++ b/single.go
@@ -1 +1 @@
-old
+new
`
result := vcs.BuildLineToPositionMap(diff)
fileMap := result["single.go"]
// @@ line -> pos 1
// "-old" -> pos 2
// "+new" -> pos 3, newLine 1
if fileMap[1] != 3 {
t.Errorf("line 1: expected position 3, got %d", fileMap[1])
}
}
func TestBuildLineToPositionMap_DeletedFile(t *testing.T) {
diff := `diff --git a/deleted.go b/deleted.go
deleted file mode 100644
--- a/deleted.go
+++ /dev/null
@@ -1,3 +0,0 @@
-package deleted
-
-func Gone() {}
`
result := vcs.BuildLineToPositionMap(diff)
if _, ok := result["deleted.go"]; ok {
t.Error("deleted file should not appear in result")
}
}