fix: address self-review findings on PR #90

- Remove unused error return from BuildPositionToLineMap (always nil)
- Add comment explaining intentional CommitID drop in PostReview
- Refactor TestAdapter_PostReview_WithComments to route by URL path
- Add TestAdapter_GetFileContent_RefRouting test
- Acknowledge maxPosition O(n) with code comment
- Remove redundant TestAdapter_CompileTimeCheck (compile-time var _ exists)
- Fix GetPullRequestFiles comment (Patch field is omitted, not 'set to empty')
- Acknowledge translateEvent fallback as intentional design
This commit is contained in:
claw
2026-05-12 13:49:36 -07:00
parent 928e2fa182
commit 701f0bed64
4 changed files with 77 additions and 67 deletions
+13 -40
View File
@@ -20,10 +20,7 @@ index abc..def 100644
+added line
context after
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
tests := []struct {
pos int
@@ -59,10 +56,7 @@ func TestBuildPositionToLineMap_MultipleHunks(t *testing.T) {
return
}
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
tests := []struct {
pos int
@@ -100,10 +94,7 @@ func TestBuildPositionToLineMap_DeletionTargeted(t *testing.T) {
-deleted
line3
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
// Position 3 is the deletion line "-deleted" — should map to nearest below
// Position 4 is " line3" which is new line 2
@@ -126,12 +117,9 @@ func TestBuildPositionToLineMap_DeletionAtEnd(t *testing.T) {
line2
-deleted at end
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
_, err = pm.Translate("file.go", 4)
_, err := pm.Translate("file.go", 4)
if err == nil {
t.Error("expected error for deletion at end with no subsequent line")
}
@@ -147,10 +135,7 @@ new file mode 100644
+
+func init() {}
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
tests := []struct {
pos int
@@ -182,13 +167,10 @@ deleted file mode 100644
-
-func old() {}
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
// Deleted file has no new-file lines; positions should error
_, err = pm.Translate("old.go", 2)
_, err := pm.Translate("old.go", 2)
if err == nil {
t.Error("expected error for deleted file position")
}
@@ -205,13 +187,10 @@ diff --git a/code.go b/code.go
+// added
func main() {}
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
// Binary file should not be in the map
_, err = pm.Translate("image.png", 1)
_, err := pm.Translate("image.png", 1)
if err == nil {
t.Error("expected error for binary file")
}
@@ -235,13 +214,10 @@ func TestBuildPositionToLineMap_OutOfRange(t *testing.T) {
-old
+new
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
// Position 0 is invalid
_, err = pm.Translate("file.go", 0)
_, err := pm.Translate("file.go", 0)
if err == nil {
t.Error("expected error for position 0")
}
@@ -275,10 +251,7 @@ diff --git a/b.go b/b.go
+// file b
func bFunc() {}
`
pm, err := BuildPositionToLineMap(diff)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
pm := BuildPositionToLineMap(diff)
// a.go: pos 3 is "+// file a" -> new line 2
got, err := pm.Translate("a.go", 3)