diff --git a/cmd/review-bot/integration_test.go b/cmd/review-bot/integration_test.go index e357624..862d002 100644 --- a/cmd/review-bot/integration_test.go +++ b/cmd/review-bot/integration_test.go @@ -10,6 +10,7 @@ import ( "testing" "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/github" "gitea.weiker.me/rodin/review-bot/llm" "gitea.weiker.me/rodin/review-bot/review" ) @@ -159,3 +160,89 @@ func TestIntegration_PostAndCleanup(t *testing.T) { t.Logf("Warning: could not delete test review %d: %v", posted.ID, err) } } + +// TestIntegration_GitHub_PostAndVerifyReview exercises the full VCS routing path +// for GitHub when INTEGRATION_GITHUB_TOKEN and INTEGRATION_GITHUB_REPO are set. +// It verifies that the GitHub adapter is selected via VCS_TYPE=github and that +// PostReview succeeds against a real GitHub PR. +// +// Required environment variables: +// +// INTEGRATION_GITHUB_TOKEN - GitHub personal access token with repo access +// INTEGRATION_GITHUB_REPO - owner/repo with an open PR (e.g. Rodin-AI/review-bot) +// INTEGRATION_GITHUB_PR - PR number to test against +// +// The test skips gracefully when these variables are absent. +func TestIntegration_GitHub_PostAndVerifyReview(t *testing.T) { + githubToken := os.Getenv("INTEGRATION_GITHUB_TOKEN") + githubRepo := os.Getenv("INTEGRATION_GITHUB_REPO") + prNumStr := os.Getenv("INTEGRATION_GITHUB_PR") + + if githubToken == "" || githubRepo == "" || prNumStr == "" { + t.Skip("INTEGRATION_GITHUB_TOKEN, INTEGRATION_GITHUB_REPO, and INTEGRATION_GITHUB_PR not set, skipping") + } + + prNumber, err := strconv.Atoi(prNumStr) + if err != nil { + t.Fatalf("Invalid PR number %q: %v", prNumStr, err) + } + + parts := strings.SplitN(githubRepo, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + t.Fatalf("Invalid repo format %q, expected owner/repo", githubRepo) + } + owner, repoName := parts[0], parts[1] + + ctx := context.Background() + + // Verify the GitHub adapter is selected via VCS routing. + t.Setenv("VCS_TYPE", "github") + + ghClient := github.NewClient(githubToken, "https://api.github.com") + + // Verify adapter selection: GetAuthenticatedUser must succeed. + user, err := ghClient.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v — check INTEGRATION_GITHUB_TOKEN", err) + } + t.Logf("Authenticated as: %s", user) + + // Verify PR is accessible via GitHub adapter. + pr, err := ghClient.GetPullRequest(ctx, owner, repoName, prNumber) + if err != nil { + t.Fatalf("GetPullRequest: %v", err) + } + t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha) + + // Post a COMMENT review — does not require PR approval permissions. + sentinel := "" + testBody := "# Integration Test Review (GitHub)\n\nThis is an automated integration test.\n\n" + sentinel + posted, err := ghClient.PostReview(ctx, owner, repoName, prNumber, "COMMENT", testBody, "", nil) + if err != nil { + t.Fatalf("PostReview: %v", err) + } + t.Logf("Posted review ID: %d", posted.ID) + + // Verify the review appears in ListReviews. + reviews, err := ghClient.ListReviews(ctx, owner, repoName, prNumber) + if err != nil { + t.Fatalf("ListReviews: %v", err) + } + found := false + for _, r := range reviews { + if r.ID == posted.ID && strings.Contains(r.Body, sentinel) { + found = true + break + } + } + if !found { + t.Errorf("posted review ID %d not found in ListReviews output", posted.ID) + } + + // Attempt cleanup — GitHub does not allow deleting submitted reviews, + // so this is expected to fail with ErrCannotDeleteSubmittedReview (422). + // Log it as informational only. + if err := ghClient.DeleteReview(ctx, owner, repoName, prNumber, posted.ID); err != nil { + t.Logf("Note: DeleteReview returned (expected for submitted GitHub reviews): %v", err) + } +} diff --git a/cmd/review-bot/main_test.go b/cmd/review-bot/main_test.go index ed458b6..989ab65 100644 --- a/cmd/review-bot/main_test.go +++ b/cmd/review-bot/main_test.go @@ -630,6 +630,49 @@ func TestEvaluateCIStatus(t *testing.T) { } } +func TestGithubAPIURL(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "empty string defaults to api.github.com", + input: "", + want: "https://api.github.com", + }, + { + name: "github.com maps to api.github.com", + input: "https://github.com", + want: "https://api.github.com", + }, + { + name: "github.com with trailing slash maps to api.github.com", + input: "https://github.com/", + want: "https://api.github.com", + }, + { + name: "GHES host gets /api/v3 suffix", + input: "https://ghe.example.com", + want: "https://ghe.example.com/api/v3", + }, + { + name: "GHES concur domain does not map to api.github.com", + input: "https://github.concur.com", + want: "https://github.concur.com/api/v3", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := githubAPIURL(tt.input) + if got != tt.want { + t.Errorf("githubAPIURL(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + + func TestEnvOrDefault(t *testing.T) { // Test with unset env var os.Unsetenv("TEST_ENV_OR_DEFAULT_UNSET") @@ -970,7 +1013,7 @@ func TestMainSubprocess_InvalidProvider(t *testing.T) { } } -// cleanEnv returns environ without any GITEA/LLM/REVIEWER env vars that would +// cleanEnv returns environ without any GITEA/LLM/REVIEWER/VCS env vars that would // interfere with testing missing-flag scenarios. func cleanEnv() []string { var env []string @@ -985,7 +1028,8 @@ func cleanEnv() []string { strings.HasPrefix(key, "CONVENTIONS_"), strings.HasPrefix(key, "SYSTEM_PROMPT_"), strings.HasPrefix(key, "PATTERNS_"), - strings.HasPrefix(key, "UPDATE_"): + strings.HasPrefix(key, "UPDATE_"), + strings.HasPrefix(key, "VCS_"): continue default: env = append(env, e)