rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

refactor(#154): extract baseSubprocessArgs helper in main_test.go subprocess tests
CI / test (pull_request) Successful in 17s
Details
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 25s
Details
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 44s
Details
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m40s
Details

Browse Source 
Reduces duplication across 7 subprocess test functions that repeated the
same set of required flags verbatim. When required flags change, only
baseSubprocessArgs() needs updating.

Changes:
- Add baseSubprocessArgs() helper (analogous to cleanEnv())
- InvalidReviewerName, InvalidTemperature, InvalidProvider,
  ConflictingPersonaFlags: use append(baseSubprocessArgs(), ...)
- InvalidRepo: mutate a copy from baseSubprocessArgs() to set invalid --repo
- InvalidPRNumber: mutate a copy from baseSubprocessArgs() to set invalid --pr
- MissingLLMBaseURL, MissingAICoreCredentials, DeprecatedGiteaURLEnv:
  keep inline args (intentionally omit base flags); add comment explaining why
- All 5 old tests migrated from deprecated --gitea-url to canonical --vcs-url

All subprocess tests pass. go vet clean.
This commit is contained in:
Rodin
2026-05-15 01:50:15 -07:00
parent 30fe48d265
commit e718cb841a
1 changed files with 50 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/review-bot/main_test.go
+50 -56
View File
@@ -880,16 +880,9 @@ func TestMainSubprocess_MissingFlags(t *testing.T) {
func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
os.Args = append(baseSubprocessArgs(),
"--reviewer-name", "invalid name",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
}
)
main()
return
}
@@ -908,15 +901,15 @@ func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
func TestMainSubprocess_InvalidRepo(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "invalidrepo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
args := baseSubprocessArgs()
// Replace the canonical --repo with an invalid one.
for i, a := range args {
if a == "owner/repo" {
args[i] = "invalidrepo"
break
}
}
os.Args = args
main()
return
}
@@ -935,15 +928,15 @@ func TestMainSubprocess_InvalidRepo(t *testing.T) {
func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "notanumber",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
args := baseSubprocessArgs()
// Replace the canonical --pr value with a non-numeric string.
for i, a := range args {
if a == "1" {
args[i] = "notanumber"
break
}
}
os.Args = args
main()
return
}
@@ -962,16 +955,9 @@ func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
func TestMainSubprocess_InvalidTemperature(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
os.Args = append(baseSubprocessArgs(),
"--llm-temperature", "5.0",
}
)
main()
return
}
@@ -990,16 +976,9 @@ func TestMainSubprocess_InvalidTemperature(t *testing.T) {
func TestMainSubprocess_InvalidProvider(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
os.Args = append(baseSubprocessArgs(),
"--llm-provider", "invalid-provider",
}
)
main()
return
}
@@ -1015,6 +994,25 @@ func TestMainSubprocess_InvalidProvider(t *testing.T) {
}
}
// baseSubprocessArgs returns the base set of required flags for subprocess tests
// that need a fully-configured main() invocation. Each test appends its own
// test-specific flags on top of this base.
//
// Using a helper here means that when the set of required flags changes, only
// this function needs updating (instead of every test that passes all flags).
func baseSubprocessArgs() []string {
return []string{
"review-bot",
"--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "https://api.example.com",
"--llm-api-key", "key",
"--llm-model", "gpt-4",
}
}
// cleanEnv returns environ without any GITEA/LLM/REVIEWER/VCS env vars that would
// interfere with testing missing-flag scenarios.
func cleanEnv() []string {
@@ -1389,13 +1387,14 @@ func TestFetchPatterns_MultipleRepos(t *testing.T) {
func TestMainSubprocess_MissingLLMBaseURL(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
// Note: cannot use baseSubprocessArgs() here because --llm-base-url and
// --llm-api-key are intentionally omitted to test the missing-URL error.
os.Args = []string{"review-bot",
"--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-model", "gpt-4",
// --llm-base-url and --llm-api-key intentionally omitted
}
main()
return
@@ -1417,6 +1416,8 @@ func TestMainSubprocess_MissingLLMBaseURL(t *testing.T) {
func TestMainSubprocess_MissingAICoreCredentials(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
// Note: cannot use baseSubprocessArgs() here because aicore provider
// does not require --llm-base-url / --llm-api-key; those are omitted.
os.Args = []string{"review-bot",
"--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo",
@@ -1446,17 +1447,10 @@ func TestMainSubprocess_MissingAICoreCredentials(t *testing.T) {
func TestMainSubprocess_ConflictingPersonaFlags(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot",
"--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "https://api.example.com",
"--llm-api-key", "key",
"--llm-model", "gpt-4",
os.Args = append(baseSubprocessArgs(),
"--persona", "security",
"--persona-file", "custom.json",
}
)
main()
return
}
@@ -1477,9 +1471,9 @@ func TestMainSubprocess_ConflictingPersonaFlags(t *testing.T) {
func TestMainSubprocess_DeprecatedGiteaURLEnv(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
// Set required flags but omit --vcs-url; GITEA_URL should be picked up.
// The test will exit with an error after VCS init (no PR to fetch), but
// the deprecation warning must appear.
// Note: cannot use baseSubprocessArgs() here because --vcs-url must be
// omitted — this test verifies that GITEA_URL env var is picked up as a
// deprecated fallback when --vcs-url is absent.
os.Args = []string{"review-bot",
// No --vcs-url: should fall back to GITEA_URL env var
"--repo", "owner/repo",