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
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:
+50
-56
@@ -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",
|
||||||
|
|||||||
Reference in New Issue
Block a user