[dev-loop] Add tests for GetTimelineReviewCommentIDForReview and GitHub GetAllFilesInPath
CI / test (push) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped

gitea: Add 4 tests for GetTimelineReviewCommentIDForReview (was 0% coverage):
- Success: find review in timeline by user login + body prefix match
- ReviewFetchError: 404 on review API
- EmptyBody: review with empty body returns error
- NotFoundInTimeline: body matches but user login doesn't

github: Add 3 tests for GetAllFilesInPath (was 0% coverage):
- DirectoryWithFiles: lists directory, fetches base64-encoded file content
- 404FallsBackToFile: 404 on dir path returns error when file also 404s
- DirectoryWithSubdir: recursive directory traversal

Coverage changes:
- gitea: 80.0% → 85.2%
- github: 79.9% → 86.3%
This commit is contained in:
Rodin
2026-05-14 23:11:47 +00:00
parent fccfdd2ff7
commit 6f02cef662
2 changed files with 172 additions and 0 deletions
+77
View File
@@ -1420,3 +1420,80 @@ func TestNewSafeHTTPClient_PreservesDefaultTransportSettings(t *testing.T) {
t.Error("DialContext is nil; expected safeDialContext")
}
}
func TestGetTimelineReviewCommentIDForReview(t *testing.T) {
const reviewID = int64(42)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/repos/owner/repo/pulls/5/reviews/42":
w.Write([]byte(`{"body": "The review body <!-- review-bot:sonnet -->", "user": {"login": "sonnet-review"}}`))
case "/api/v1/repos/owner/repo/issues/5/timeline":
w.Write([]byte(`[
{"id": 100, "type": "comment", "body": "unrelated", "user": {"login": "sonnet-review"}},
{"id": 200, "type": "review", "body": "The review body <!-- review-bot:sonnet -->", "user": {"login": "sonnet-review"}}
]`))
default:
t.Errorf("unexpected path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer server.Close()
client := NewTestClient(server.URL, "test-token")
id, err := client.GetTimelineReviewCommentIDForReview(context.Background(), "owner", "repo", 5, reviewID)
if err != nil {
t.Fatalf("GetTimelineReviewCommentIDForReview() error = %v", err)
}
if id != 200 {
t.Errorf("got id=%d, want 200", id)
}
}
func TestGetTimelineReviewCommentIDForReview_ReviewFetchError(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"message":"not found"}`))
}))
defer server.Close()
client := NewTestClient(server.URL, "test-token")
_, err := client.GetTimelineReviewCommentIDForReview(context.Background(), "owner", "repo", 5, 99)
if err == nil {
t.Fatal("expected error for missing review, got nil")
}
}
func TestGetTimelineReviewCommentIDForReview_EmptyBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`{"body": "", "user": {"login": "bot"}}`))
}))
defer server.Close()
client := NewTestClient(server.URL, "test-token")
_, err := client.GetTimelineReviewCommentIDForReview(context.Background(), "owner", "repo", 5, 42)
if err == nil {
t.Fatal("expected error for empty body, got nil")
}
if !strings.Contains(err.Error(), "empty body") {
t.Errorf("error = %q, want to contain 'empty body'", err.Error())
}
}
func TestGetTimelineReviewCommentIDForReview_NotFoundInTimeline(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/repos/owner/repo/pulls/5/reviews/42":
w.Write([]byte(`{"body": "review content <!-- review-bot:sonnet -->", "user": {"login": "bot"}}`))
default:
// Timeline returns events that don't match (different user)
w.Write([]byte(`[{"id": 1, "type": "review", "body": "review content <!-- review-bot:sonnet -->", "user": {"login": "other-user"}}]`))
}
}))
defer server.Close()
client := NewTestClient(server.URL, "test-token")
_, err := client.GetTimelineReviewCommentIDForReview(context.Background(), "owner", "repo", 5, 42)
if err == nil {
t.Fatal("expected error when review not found in timeline, got nil")
}
}
+95
View File
@@ -1130,3 +1130,98 @@ func TestEscapePath_SpecialChars(t *testing.T) {
}
}
}
func TestGetAllFilesInPath_DirectoryWithFiles(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/repos/owner/repo/contents/patterns":
// Directory listing
w.WriteHeader(http.StatusOK)
w.Write([]byte(`[{"name":"go.md","path":"patterns/go.md","type":"file"}]`))
case "/repos/owner/repo/contents/patterns/go.md":
// GitHub file response with base64 content
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"name":"go.md","path":"patterns/go.md","type":"file","encoding":"base64","content":"IyBHbyBwYXR0ZXJucwo="}`))
default:
t.Errorf("unexpected path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer srv.Close()
c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
files, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "patterns")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 1 {
t.Fatalf("len(files) = %d, want 1", len(files))
}
if files["patterns/go.md"] != "# Go patterns\n" {
t.Errorf("unexpected content: %q", files["patterns/go.md"])
}
}
func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/repos/owner/repo/contents/README.md":
// ListContents returns 404 for file paths
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"message":"Not Found"}`))
default:
t.Errorf("unexpected path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer srv.Close()
c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
// GetFileContent also goes to /contents/ — this will 404 too.
// The function should return the path-not-found error.
_, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
if err == nil {
t.Fatal("expected error when both dir and file 404, got nil")
}
}
func TestGetAllFilesInPath_DirectoryWithSubdir(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/repos/owner/repo/contents/src":
w.WriteHeader(http.StatusOK)
w.Write([]byte(`[
{"name":"main.go","path":"src/main.go","type":"file"},
{"name":"sub","path":"src/sub","type":"dir"}
]`))
case "/repos/owner/repo/contents/src/main.go":
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"name":"main.go","path":"src/main.go","type":"file","encoding":"base64","content":"cGFja2FnZSBtYWluCg=="}`))
case "/repos/owner/repo/contents/src/sub":
w.WriteHeader(http.StatusOK)
w.Write([]byte(`[{"name":"util.go","path":"src/sub/util.go","type":"file"}]`))
case "/repos/owner/repo/contents/src/sub/util.go":
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"name":"util.go","path":"src/sub/util.go","type":"file","encoding":"base64","content":"cGFja2FnZSBzdWIK"}`))
default:
t.Errorf("unexpected path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer srv.Close()
c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
files, err := c.GetAllFilesInPath(context.Background(), "owner", "repo", "src")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(files) != 2 {
t.Fatalf("len(files) = %d, want 2: %v", len(files), files)
}
if files["src/main.go"] != "package main\n" {
t.Errorf("src/main.go content unexpected: %q", files["src/main.go"])
}
if files["src/sub/util.go"] != "package sub\n" {
t.Errorf("src/sub/util.go content unexpected: %q", files["src/sub/util.go"])
}
}