diff --git a/cmd/opencodereview/compat_test.go b/cmd/opencodereview/compat_test.go index 886bfa7..883e619 100644 --- a/cmd/opencodereview/compat_test.go +++ b/cmd/opencodereview/compat_test.go @@ -20,35 +20,6 @@ func parseReviewFlags(args []string) (reviewOptions, error) { return opts, err } -// parseConfigArgs provides test compatibility for config argument parsing. -type configAction struct { - subCmd string - key string - value string -} - -func parseConfigArgs(args []string) (configAction, error) { - if len(args) == 0 { - return configAction{}, &configParseError{"usage: ocr config set \ne.g., ocr config set llm.model claude-opus-4-6"} - } - - subCmd := args[0] - switch subCmd { - case "set": - if len(args) < 3 { - return configAction{}, &configParseError{"usage: ocr config set \ne.g., ocr config set llm.model claude-opus-4-6"} - } - return configAction{subCmd: "set", key: args[1], value: args[2]}, nil - case "unset": - if len(args) < 2 { - return configAction{}, &configParseError{"usage: ocr config unset |mcp_servers.>\nexamples:\n ocr config unset provider\n ocr config unset custom_providers.my-provider\n ocr config unset mcp_servers.github"} - } - return configAction{subCmd: "unset", key: args[1]}, nil - default: - return configAction{}, &configParseError{"unknown config sub-command: " + subCmd + "\nAvailable: set, unset, provider, model"} - } -} - // parseScanFlags provides test compatibility: parses args through a fresh // cobra command instance and returns the resulting scanOptions. func parseScanFlags(args []string) (scanOptions, error) { @@ -68,10 +39,6 @@ func parseScanFlags(args []string) (scanOptions, error) { return opts, err } -type configParseError struct{ msg string } - -func (e *configParseError) Error() string { return e.msg } - // runSessionListCompat provides test compatibility for old-style runSessionList([]string{...}) calls. func runSessionListCompat(args []string) error { cmd := &cobra.Command{ @@ -129,38 +96,37 @@ func runSession(args []string) error { return cmd.Execute() } -// Ensure configParseError implements the error interface. -var _ error = (*configParseError)(nil) - -// runConfig provides test compatibility: dispatches config subcommands. +// runConfig provides test compatibility: dispatches config subcommands through a +// fresh cobra command tree that mirrors the production config command, so the +// test compat layer cannot drift from production routing. func runConfig(args []string) error { - if len(args) == 0 { - return nil + cmd := &cobra.Command{Use: "config", SilenceUsage: true, SilenceErrors: true} + setCmd := &cobra.Command{ + Use: "set", Args: cobra.ExactArgs(2), + SilenceUsage: true, SilenceErrors: true, + RunE: func(cmd *cobra.Command, a []string) error { return runConfigSet(a[0], a[1]) }, } - switch args[0] { - case "provider": - if len(args) != 1 { - return &configParseError{"config provider does not accept arguments; use 'ocr config set provider ' for non-interactive setup"} - } - return runConfigProvider() - case "model": - if len(args) != 1 { - return &configParseError{"config model does not accept arguments; use 'ocr config set model ' for non-interactive setup"} - } - return runConfigModel() + unsetCmd := &cobra.Command{ + Use: "unset", Args: cobra.ExactArgs(1), + SilenceUsage: true, SilenceErrors: true, + RunE: func(cmd *cobra.Command, a []string) error { return runConfigUnset(a[0]) }, } - - action, err := parseConfigArgs(args) - if err != nil { - return err + providerCmd := &cobra.Command{ + Use: "provider", Args: cobra.NoArgs, + SilenceUsage: true, SilenceErrors: true, + RunE: func(cmd *cobra.Command, a []string) error { return runConfigProvider() }, } - - switch action.subCmd { - case "set": - return runConfigSet(action.key, action.value) - case "unset": - return runConfigUnset(action.key) - default: - return &configParseError{"unknown config sub-command: " + action.subCmd} + modelCmd := &cobra.Command{ + Use: "model", Args: cobra.NoArgs, + SilenceUsage: true, SilenceErrors: true, + RunE: func(cmd *cobra.Command, a []string) error { return runConfigModel() }, } + cmd.AddCommand(setCmd, unsetCmd, providerCmd, modelCmd) + // A nil args slice makes cobra fall back to os.Args; pass an explicit empty + // slice so `runConfig(nil)` shows usage instead of reading the test binary's args. + if args == nil { + args = []string{} + } + cmd.SetArgs(args) + return cmd.Execute() } diff --git a/cmd/opencodereview/config_cmd_test.go b/cmd/opencodereview/config_cmd_test.go index f2b4b24..218bea6 100644 --- a/cmd/opencodereview/config_cmd_test.go +++ b/cmd/opencodereview/config_cmd_test.go @@ -369,25 +369,9 @@ func TestSetConfigValueCustomProviderExtraHeaders(t *testing.T) { // --- unset tests --- -func TestParseConfigArgsUnset(t *testing.T) { - action, err := parseConfigArgs([]string{"unset", "custom_providers.my-gateway"}) - if err != nil { - t.Fatalf("parseConfigArgs: %v", err) - } - if action.subCmd != "unset" { - t.Errorf("subCmd = %q, want %q", action.subCmd, "unset") - } - if action.key != "custom_providers.my-gateway" { - t.Errorf("key = %q, want %q", action.key, "custom_providers.my-gateway") - } -} - -func TestParseConfigArgsUnsetMissingKey(t *testing.T) { - _, err := parseConfigArgs([]string{"unset"}) - if err == nil { - t.Fatal("expected error for missing key") - } -} +// parseConfigArgs unset tests were removed with the helper; unset dispatch is +// covered by the TestRunConfigUnset_* tests and config_dispatch_test.go, and the +// deeper unset behavior by the TestDeleteCustomProvider / provider tests below. func TestUnsetCustomProvider(t *testing.T) { dir := t.TempDir() diff --git a/cmd/opencodereview/flags_test.go b/cmd/opencodereview/flags_test.go index d928117..e35f9fc 100644 --- a/cmd/opencodereview/flags_test.go +++ b/cmd/opencodereview/flags_test.go @@ -147,50 +147,5 @@ func TestParseReviewFlags_ShortFlags(t *testing.T) { } } -func TestParseConfigArgs_Empty(t *testing.T) { - _, err := parseConfigArgs(nil) - if err == nil { - t.Fatal("expected error for empty args") - } -} - -func TestParseConfigArgs_Set(t *testing.T) { - act, err := parseConfigArgs([]string{"set", "llm.model", "gpt-4"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if act.subCmd != "set" || act.key != "llm.model" || act.value != "gpt-4" { - t.Errorf("got %+v", act) - } -} - -func TestParseConfigArgs_SetMissingValue(t *testing.T) { - _, err := parseConfigArgs([]string{"set", "llm.model"}) - if err == nil { - t.Fatal("expected error for missing value") - } -} - -func TestParseConfigArgs_Unset(t *testing.T) { - act, err := parseConfigArgs([]string{"unset", "custom_providers.foo"}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if act.subCmd != "unset" || act.key != "custom_providers.foo" { - t.Errorf("got %+v", act) - } -} - -func TestParseConfigArgs_UnsetMissingKey(t *testing.T) { - _, err := parseConfigArgs([]string{"unset"}) - if err == nil { - t.Fatal("expected error for missing key") - } -} - -func TestParseConfigArgs_UnknownSubCmd(t *testing.T) { - _, err := parseConfigArgs([]string{"delete", "foo"}) - if err == nil { - t.Fatal("expected error for unknown subcommand") - } -} +// Config argument parsing is exercised through runConfig (see +// config_dispatch_test.go), which now routes via the production cobra tree.