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

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
+50 -56
View File
@@ -880,16 +880,9 @@ func TestMainSubprocess_MissingFlags(t *testing.T) {
func TestMainSubprocess_InvalidReviewerName(t *testing.T) { func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", os.Args = append(baseSubprocessArgs(),
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-name", "invalid name", "--reviewer-name", "invalid name",
"--reviewer-token", "tok", )
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
}
main() main()
return return
} }
@@ -908,15 +901,15 @@ func TestMainSubprocess_InvalidReviewerName(t *testing.T) {
func TestMainSubprocess_InvalidRepo(t *testing.T) { func TestMainSubprocess_InvalidRepo(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", args := baseSubprocessArgs()
"--gitea-url", "http://localhost", // Replace the canonical --repo with an invalid one.
"--repo", "invalidrepo", for i, a := range args {
"--pr", "1", if a == "owner/repo" {
"--reviewer-token", "tok", args[i] = "invalidrepo"
"--llm-base-url", "http://localhost", break
"--llm-api-key", "key", }
"--llm-model", "model",
} }
os.Args = args
main() main()
return return
} }
@@ -935,15 +928,15 @@ func TestMainSubprocess_InvalidRepo(t *testing.T) {
func TestMainSubprocess_InvalidPRNumber(t *testing.T) { func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", args := baseSubprocessArgs()
"--gitea-url", "http://localhost", // Replace the canonical --pr value with a non-numeric string.
"--repo", "owner/repo", for i, a := range args {
"--pr", "notanumber", if a == "1" {
"--reviewer-token", "tok", args[i] = "notanumber"
"--llm-base-url", "http://localhost", break
"--llm-api-key", "key", }
"--llm-model", "model",
} }
os.Args = args
main() main()
return return
} }
@@ -962,16 +955,9 @@ func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
func TestMainSubprocess_InvalidTemperature(t *testing.T) { func TestMainSubprocess_InvalidTemperature(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", os.Args = append(baseSubprocessArgs(),
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
"--llm-temperature", "5.0", "--llm-temperature", "5.0",
} )
main() main()
return return
} }
@@ -990,16 +976,9 @@ func TestMainSubprocess_InvalidTemperature(t *testing.T) {
func TestMainSubprocess_InvalidProvider(t *testing.T) { func TestMainSubprocess_InvalidProvider(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", os.Args = append(baseSubprocessArgs(),
"--gitea-url", "http://localhost",
"--repo", "owner/repo",
"--pr", "1",
"--reviewer-token", "tok",
"--llm-base-url", "http://localhost",
"--llm-api-key", "key",
"--llm-model", "model",
"--llm-provider", "invalid-provider", "--llm-provider", "invalid-provider",
} )
main() main()
return 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 // cleanEnv returns environ without any GITEA/LLM/REVIEWER/VCS env vars that would
// interfere with testing missing-flag scenarios. // interfere with testing missing-flag scenarios.
func cleanEnv() []string { func cleanEnv() []string {
@@ -1389,13 +1387,14 @@ func TestFetchPatterns_MultipleRepos(t *testing.T) {
func TestMainSubprocess_MissingLLMBaseURL(t *testing.T) { func TestMainSubprocess_MissingLLMBaseURL(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) 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", os.Args = []string{"review-bot",
"--vcs-url", "https://gitea.example.com", "--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo", "--repo", "owner/repo",
"--pr", "1", "--pr", "1",
"--reviewer-token", "tok", "--reviewer-token", "tok",
"--llm-model", "gpt-4", "--llm-model", "gpt-4",
// --llm-base-url and --llm-api-key intentionally omitted
} }
main() main()
return return
@@ -1417,6 +1416,8 @@ func TestMainSubprocess_MissingLLMBaseURL(t *testing.T) {
func TestMainSubprocess_MissingAICoreCredentials(t *testing.T) { func TestMainSubprocess_MissingAICoreCredentials(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) 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", os.Args = []string{"review-bot",
"--vcs-url", "https://gitea.example.com", "--vcs-url", "https://gitea.example.com",
"--repo", "owner/repo", "--repo", "owner/repo",
@@ -1446,17 +1447,10 @@ func TestMainSubprocess_MissingAICoreCredentials(t *testing.T) {
func TestMainSubprocess_ConflictingPersonaFlags(t *testing.T) { func TestMainSubprocess_ConflictingPersonaFlags(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
os.Args = []string{"review-bot", os.Args = append(baseSubprocessArgs(),
"--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",
"--persona", "security", "--persona", "security",
"--persona-file", "custom.json", "--persona-file", "custom.json",
} )
main() main()
return return
} }
@@ -1477,9 +1471,9 @@ func TestMainSubprocess_ConflictingPersonaFlags(t *testing.T) {
func TestMainSubprocess_DeprecatedGiteaURLEnv(t *testing.T) { func TestMainSubprocess_DeprecatedGiteaURLEnv(t *testing.T) {
if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" { if os.Getenv("TEST_SUBPROCESS_MAIN") == "1" {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
// Set required flags but omit --vcs-url; GITEA_URL should be picked up. // Note: cannot use baseSubprocessArgs() here because --vcs-url must be
// The test will exit with an error after VCS init (no PR to fetch), but // omitted — this test verifies that GITEA_URL env var is picked up as a
// the deprecation warning must appear. // deprecated fallback when --vcs-url is absent.
os.Args = []string{"review-bot", os.Args = []string{"review-bot",
// No --vcs-url: should fall back to GITEA_URL env var // No --vcs-url: should fall back to GITEA_URL env var
"--repo", "owner/repo", "--repo", "owner/repo",