diff --git a/cmd/opencodereview/config_cmd_test.go b/cmd/opencodereview/config_cmd_test.go index 88f1f79..af1e43a 100644 --- a/cmd/opencodereview/config_cmd_test.go +++ b/cmd/opencodereview/config_cmd_test.go @@ -1,6 +1,7 @@ package main import ( + "os" "testing" ) @@ -417,6 +418,379 @@ func TestUnsetInvalidKey(t *testing.T) { } } +func TestMergeModelLists(t *testing.T) { + tests := []struct { + name string + lists [][]string + want []string + }{ + {"empty", nil, nil}, + {"single list", [][]string{{"a", "b"}}, []string{"a", "b"}}, + {"merge with dedup", [][]string{{"a", "b"}, {"b", "c"}}, []string{"a", "b", "c"}}, + {"three lists", [][]string{{"x"}, {"y"}, {"x", "z"}}, []string{"x", "y", "z"}}, + {"empty strings filtered", [][]string{{"a", "", "b"}}, []string{"a", "b"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := mergeModelLists(tc.lists...) + if len(got) != len(tc.want) { + t.Fatalf("mergeModelLists() = %v, want %v", got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("[%d] = %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestEnsureTelemetry(t *testing.T) { + cfg := &Config{} + if cfg.Telemetry != nil { + t.Fatal("Telemetry should be nil initially") + } + cfg.ensureTelemetry() + if cfg.Telemetry == nil { + t.Fatal("Telemetry should be non-nil after ensureTelemetry()") + } + cfg.ensureTelemetry() + if cfg.Telemetry == nil { + t.Fatal("Telemetry should remain non-nil on second call") + } +} + +func TestSetConfigValueLlmURL(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.url", "https://example.com/v1"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Llm.URL != "https://example.com/v1" { + t.Errorf("URL = %q", cfg.Llm.URL) + } +} + +func TestSetConfigValueLlmAuthToken(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.auth_token", "tok-123"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Llm.AuthToken != "tok-123" { + t.Errorf("AuthToken = %q", cfg.Llm.AuthToken) + } +} + +func TestSetConfigValueLlmModel(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.model", "my-model"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Llm.Model != "my-model" { + t.Errorf("Model = %q", cfg.Llm.Model) + } +} + +func TestSetConfigValueLlmUseAnthropic(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.use_anthropic", "false"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Llm.UseAnthropic == nil || *cfg.Llm.UseAnthropic != false { + t.Errorf("UseAnthropic = %v", cfg.Llm.UseAnthropic) + } +} + +func TestSetConfigValueLlmUseAnthropicInvalid(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.use_anthropic", "notbool"); err == nil { + t.Fatal("expected error for invalid boolean") + } +} + +func TestSetConfigValueLanguage(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "language", "English"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Language != "English" { + t.Errorf("Language = %q", cfg.Language) + } +} + +func TestSetConfigValueTelemetryEnabled(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.enabled", "true"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Telemetry == nil || !cfg.Telemetry.Enabled { + t.Error("Telemetry.Enabled should be true") + } +} + +func TestSetConfigValueTelemetryEnabledInvalid(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.enabled", "notbool"); err == nil { + t.Fatal("expected error for invalid boolean") + } +} + +func TestSetConfigValueTelemetryExporter(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.exporter", "otlp"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Telemetry.Exporter != "otlp" { + t.Errorf("Exporter = %q", cfg.Telemetry.Exporter) + } +} + +func TestSetConfigValueTelemetryOTLPEndpoint(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.otlp_endpoint", "localhost:4317"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Telemetry.OTLPEndpoint != "localhost:4317" { + t.Errorf("OTLPEndpoint = %q", cfg.Telemetry.OTLPEndpoint) + } +} + +func TestSetConfigValueTelemetryContentLogging(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.content_logging", "true"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if !cfg.Telemetry.ContentLog { + t.Error("ContentLog should be true") + } +} + +func TestSetConfigValueTelemetryContentLoggingInvalid(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "telemetry.content_logging", "notbool"); err == nil { + t.Fatal("expected error for invalid boolean") + } +} + +func TestSetConfigValueLlmExtraBody(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.extra_body", `{"key":"val"}`); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Llm.ExtraBody == nil { + t.Fatal("ExtraBody should not be nil") + } + if cfg.Llm.ExtraBody["key"] != "val" { + t.Errorf("ExtraBody[\"key\"] = %v", cfg.Llm.ExtraBody["key"]) + } +} + +func TestSetConfigValueLlmExtraBodyInvalid(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "llm.extra_body", "not-json"); err == nil { + t.Fatal("expected error for invalid JSON") + } +} + +func TestSetConfigValueUnknownKey(t *testing.T) { + cfg := &Config{} + if err := setConfigValue(cfg, "unknown.key", "val"); err == nil { + t.Fatal("expected error for unknown key") + } +} + +func TestSetConfigValueProviderClearsModel(t *testing.T) { + cfg := &Config{Provider: "old-provider", Model: "old-model"} + if err := setConfigValue(cfg, "provider", "new-provider"); err != nil { + t.Fatalf("setConfigValue: %v", err) + } + if cfg.Model != "" { + t.Errorf("Model should be cleared on provider change, got %q", cfg.Model) + } +} + +func TestRunConfigUnset_InvalidKey(t *testing.T) { + if err := runConfigUnset("provider"); err == nil { + t.Fatal("expected error for non custom_providers key") + } + if err := runConfigUnset("custom_providers."); err == nil { + t.Fatal("expected error for empty provider name") + } +} + +func TestRunConfig_EmptyArgs(t *testing.T) { + err := runConfig(nil) + if err != nil { + t.Fatalf("runConfig with nil args should print usage, got error: %v", err) + } +} + +func TestRunConfig_ProviderWithArgs(t *testing.T) { + err := runConfig([]string{"provider", "extra"}) + if err == nil { + t.Fatal("expected error when provider has args") + } +} + +func TestRunConfig_ModelWithArgs(t *testing.T) { + err := runConfig([]string{"model", "extra"}) + if err == nil { + t.Fatal("expected error when model has args") + } +} + +func TestDeleteCustomProvider_NotFound(t *testing.T) { + cfg := &Config{} + _, err := deleteCustomProvider(cfg, "nonexistent") + if err == nil { + t.Fatal("expected error for nil CustomProviders") + } + + cfg.CustomProviders = map[string]ProviderEntry{"other": {}} + _, err = deleteCustomProvider(cfg, "nonexistent") + if err == nil { + t.Fatal("expected error for missing provider") + } +} + +func TestActiveModelForProvider(t *testing.T) { + tests := []struct { + name string + cfg *Config + provider string + entry ProviderEntry + want string + }{ + {"entry model", nil, "p", ProviderEntry{Model: "m1"}, "m1"}, + {"cfg model", &Config{Provider: "p", Model: "m2"}, "p", ProviderEntry{}, "m2"}, + {"entry takes precedence", &Config{Provider: "p", Model: "m2"}, "p", ProviderEntry{Model: "m1"}, "m1"}, + {"different provider", &Config{Provider: "other", Model: "m2"}, "p", ProviderEntry{}, ""}, + {"no model", &Config{Provider: "p"}, "p", ProviderEntry{}, ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := activeModelForProvider(tc.cfg, tc.provider, tc.entry) + if got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestNormalizeModelList(t *testing.T) { + tests := []struct { + name string + models []string + want []string + }{ + {"dedup", []string{"a", "b", "a"}, []string{"a", "b"}}, + {"trim spaces", []string{" a ", " b "}, []string{"a", "b"}}, + {"filter empty", []string{"a", "", "b"}, []string{"a", "b"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := normalizeModelList(tc.models) + if len(got) != len(tc.want) { + t.Fatalf("got %v, want %v", got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("[%d] = %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestParseModelListValue(t *testing.T) { + tests := []struct { + name string + value string + want int + }{ + {"empty", "", 0}, + {"json array", `["a","b"]`, 2}, + {"comma separated", "a,b,c", 3}, + {"bracket unquoted", "[a,b]", 2}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := parseModelListValue(tc.value) + if err != nil { + t.Fatalf("parseModelListValue: %v", err) + } + if len(got) != tc.want { + t.Errorf("got %d models, want %d: %v", len(got), tc.want, got) + } + }) + } +} + +func TestResolveConfigPath_Default(t *testing.T) { + t.Setenv("OCR_CONFIG_PATH", "") + p, err := resolveConfigPath() + if err != nil { + t.Fatalf("resolveConfigPath: %v", err) + } + if p == "" { + t.Fatal("expected non-empty default config path") + } +} + +func TestResolveConfigPath_Env(t *testing.T) { + t.Setenv("OCR_CONFIG_PATH", "/tmp/test-config.json") + p, err := resolveConfigPath() + if err != nil { + t.Fatalf("resolveConfigPath: %v", err) + } + if p != "/tmp/test-config.json" { + t.Errorf("path = %q, want /tmp/test-config.json", p) + } +} + +func TestLoadOrCreateConfig_NewFile(t *testing.T) { + cfg, err := loadOrCreateConfig(t.TempDir() + "/nonexistent.json") + if err != nil { + t.Fatalf("loadOrCreateConfig: %v", err) + } + if cfg == nil { + t.Fatal("expected non-nil config") + } +} + +func TestLoadOrCreateConfig_InvalidJSON(t *testing.T) { + dir := t.TempDir() + path := dir + "/bad.json" + if err := os.WriteFile(path, []byte("{invalid"), 0644); err != nil { + t.Fatal(err) + } + _, err := loadOrCreateConfig(path) + if err == nil { + t.Fatal("expected error for invalid JSON") + } +} + +func TestLoadAppConfig_NotExist(t *testing.T) { + cfg, err := LoadAppConfig(t.TempDir() + "/none.json") + if err != nil { + t.Fatalf("LoadAppConfig: %v", err) + } + if cfg != nil { + t.Fatal("expected nil config for non-existent file") + } +} + +func TestLoadAppConfig_InvalidJSON(t *testing.T) { + dir := t.TempDir() + path := dir + "/bad.json" + if err := os.WriteFile(path, []byte("not json"), 0644); err != nil { + t.Fatal(err) + } + _, err := LoadAppConfig(path) + if err == nil { + t.Fatal("expected error for invalid JSON") + } +} + func TestEnsureModelInList(t *testing.T) { models := []string{"test-model", "test-model-2", "bbb", "aaa", "test-model-3"} diff --git a/cmd/opencodereview/config_dispatch_test.go b/cmd/opencodereview/config_dispatch_test.go new file mode 100644 index 0000000..87f02b7 --- /dev/null +++ b/cmd/opencodereview/config_dispatch_test.go @@ -0,0 +1,30 @@ +package main + +import ( + "strings" + "testing" +) + +func TestRunConfig_UnknownSubcommand(t *testing.T) { + err := runConfig([]string{"delete", "foo"}) + if err == nil { + t.Fatal("expected error for unknown subcommand") + } + if !strings.Contains(err.Error(), "unknown") { + t.Errorf("error = %q, expected to contain 'unknown'", err.Error()) + } +} + +func TestRunConfig_InvalidSetMissingValue(t *testing.T) { + err := runConfig([]string{"set", "provider"}) + if err == nil { + t.Fatal("expected error for set without value") + } +} + +func TestRunConfig_InvalidUnsetMissingKey(t *testing.T) { + err := runConfig([]string{"unset"}) + if err == nil { + t.Fatal("expected error for unset without key") + } +} diff --git a/cmd/opencodereview/emit_run_result_test.go b/cmd/opencodereview/emit_run_result_test.go new file mode 100644 index 0000000..4082ca2 --- /dev/null +++ b/cmd/opencodereview/emit_run_result_test.go @@ -0,0 +1,177 @@ +package main + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" + + "github.com/open-code-review/open-code-review/internal/agent" + "github.com/open-code-review/open-code-review/internal/model" +) + +type mockResultProvider struct { + diffs []model.Diff + filesReviewed int64 + inputTokens int64 + outputTokens int64 + totalTokens int64 + cacheReadTokens int64 + cacheWriteTokens int64 + warnings []agent.AgentWarning + projectSummary string + toolCalls map[string]int64 +} + +func (m *mockResultProvider) Diffs() []model.Diff { return m.diffs } +func (m *mockResultProvider) FilesReviewed() int64 { return m.filesReviewed } +func (m *mockResultProvider) TotalInputTokens() int64 { return m.inputTokens } +func (m *mockResultProvider) TotalOutputTokens() int64 { return m.outputTokens } +func (m *mockResultProvider) TotalTokensUsed() int64 { return m.totalTokens } +func (m *mockResultProvider) TotalCacheReadTokens() int64 { return m.cacheReadTokens } +func (m *mockResultProvider) TotalCacheWriteTokens() int64 { return m.cacheWriteTokens } +func (m *mockResultProvider) Warnings() []agent.AgentWarning { return m.warnings } +func (m *mockResultProvider) ProjectSummary() string { return m.projectSummary } +func (m *mockResultProvider) ToolCalls() map[string]int64 { return m.toolCalls } + +func TestEmitRunResult_JSONNoFiles(t *testing.T) { + ag := &mockResultProvider{filesReviewed: 0} + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "developer", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + var out jsonOutput + if err := json.Unmarshal([]byte(got), &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if out.Status != "skipped" { + t.Errorf("status = %q, want skipped", out.Status) + } +} + +func TestEmitRunResult_JSONWithComments(t *testing.T) { + ag := &mockResultProvider{ + filesReviewed: 3, + inputTokens: 100, + outputTokens: 50, + totalTokens: 150, + warnings: []agent.AgentWarning{{Type: "info", Message: "note"}}, + toolCalls: map[string]int64{"file_read": 2}, + } + comments := []model.LlmComment{{Path: "main.go", Content: "fix", StartLine: 1, EndLine: 2}} + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, comments, time.Now(), "json", "developer", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + var out jsonOutput + if err := json.Unmarshal([]byte(got), &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(out.Comments) != 1 { + t.Errorf("expected 1 comment, got %d", len(out.Comments)) + } + if out.Summary == nil || out.Summary.FilesReviewed != 3 { + t.Errorf("summary.FilesReviewed = %v", out.Summary) + } +} + +func TestEmitRunResult_TextNoComments(t *testing.T) { + ag := &mockResultProvider{filesReviewed: 2} + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "Looks good to me") { + t.Errorf("expected 'Looks good to me', got %q", got) + } +} + +func TestEmitRunResult_TextWithComments(t *testing.T) { + ag := &mockResultProvider{filesReviewed: 1} + comments := []model.LlmComment{{Path: "a.go", Content: "rename", StartLine: 5, EndLine: 10}} + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, comments, time.Now(), "text", "developer", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "a.go") { + t.Errorf("expected path, got %q", got) + } + if !strings.Contains(got, "rename") { + t.Errorf("expected comment content, got %q", got) + } +} + +func TestEmitRunResult_TextWithProjectSummary(t *testing.T) { + ag := &mockResultProvider{ + filesReviewed: 5, + projectSummary: "All tests pass, code quality is good.", + } + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "developer", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "Project Summary") { + t.Errorf("expected 'Project Summary', got %q", got) + } + if !strings.Contains(got, "All tests pass") { + t.Errorf("expected summary content, got %q", got) + } +} + +func TestEmitRunResult_AgentTextRestoresQuiet(t *testing.T) { + ag := &mockResultProvider{filesReviewed: 1} + q := newQuietHandle("text", "agent") + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", q) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if q.fn != nil { + t.Error("expected quiet handle to be restored") + } + _ = got +} + +func TestEmitRunResult_AgentJSONDoesNotRestore(t *testing.T) { + ag := &mockResultProvider{ + filesReviewed: 1, + inputTokens: 10, + outputTokens: 5, + totalTokens: 15, + } + q := newQuietHandle("json", "agent") + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "json", "agent", q) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + var out jsonOutput + if err := json.Unmarshal([]byte(got), &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + q.Restore() +} + +func TestEmitRunResult_NilQuietHandle(t *testing.T) { + ag := &mockResultProvider{filesReviewed: 1} + got := captureStdout(t, func() { + err := emitRunResult(context.Background(), ag, nil, time.Now(), "text", "agent", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + _ = got +} diff --git a/cmd/opencodereview/flags_test.go b/cmd/opencodereview/flags_test.go index 617dfa4..15701ca 100644 --- a/cmd/opencodereview/flags_test.go +++ b/cmd/opencodereview/flags_test.go @@ -1,6 +1,9 @@ package main -import "testing" +import ( + "testing" + "time" +) func TestParseReviewFlagsModelOverride(t *testing.T) { opts, err := parseReviewFlags([]string{"--model", "claude-opus-4-6"}) @@ -18,3 +21,174 @@ func TestParseReviewFlagsModelOverride(t *testing.T) { t.Errorf("audience = %q, want %q", opts.audience, "human") } } + +func TestParseReviewFlags_InvalidAudience(t *testing.T) { + _, err := parseReviewFlags([]string{"--audience", "robot"}) + if err == nil { + t.Fatal("expected error for invalid audience") + } +} + +func TestParseReviewFlags_NegativeMaxTools(t *testing.T) { + _, err := parseReviewFlags([]string{"--max-tools", "-1"}) + if err == nil { + t.Fatal("expected error for negative max-tools") + } +} + +func TestParseReviewFlags_MaxToolsBelowMin(t *testing.T) { + opts, err := parseReviewFlags([]string{"--max-tools", "5"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.maxTools != 10 { + t.Errorf("maxTools = %d, want 10 (clamped to min)", opts.maxTools) + } +} + +func TestParseReviewFlags_NegativeMaxGitProcs(t *testing.T) { + _, err := parseReviewFlags([]string{"--max-git-procs", "-1"}) + if err == nil { + t.Fatal("expected error for negative max-git-procs") + } +} + +func TestParseReviewFlags_ConflictingModes(t *testing.T) { + _, err := parseReviewFlags([]string{"--from", "main", "--to", "dev", "--commit", "abc"}) + if err == nil { + t.Fatal("expected error for conflicting modes") + } +} + +func TestParseReviewFlags_FromWithoutTo(t *testing.T) { + _, err := parseReviewFlags([]string{"--from", "main"}) + if err == nil { + t.Fatal("expected error for --from without --to") + } +} + +func TestParseReviewFlags_ToWithoutFrom(t *testing.T) { + _, err := parseReviewFlags([]string{"--to", "dev"}) + if err == nil { + t.Fatal("expected error for --to without --from") + } +} + +func TestParseReviewFlags_Help(t *testing.T) { + opts, err := parseReviewFlags([]string{"-h"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.showHelp { + t.Error("expected showHelp=true") + } +} + +func TestParseReviewFlags_ShortFlags(t *testing.T) { + opts, err := parseReviewFlags([]string{"-c", "abc123", "-f", "json", "-p"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.commit != "abc123" { + t.Errorf("commit = %q, want abc123", opts.commit) + } + if opts.outputFormat != "json" { + t.Errorf("outputFormat = %q, want json", opts.outputFormat) + } + if !opts.preview { + t.Error("expected preview=true") + } +} + +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") + } +} + +func TestDurationVar(t *testing.T) { + fs := newOcrFlagSet("test") + var d time.Duration + fs.DurationVar(&d, "timeout", 5*time.Second, "max duration") + if err := fs.Parse([]string{"--timeout", "10s"}); err != nil { + t.Fatalf("parse: %v", err) + } + if d != 10*time.Second { + t.Errorf("d = %v, want 10s", d) + } +} + +func TestPrintDefaults(t *testing.T) { + fs := newOcrFlagSet("test") + var s string + fs.StringVar(&s, "name", "default", "a name") + fs.PrintDefaults() +} + +func TestExpandShortFlags(t *testing.T) { + m := map[string]string{"c": "commit", "f": "format"} + tests := []struct { + name string + args []string + want []string + }{ + {"expands short", []string{"-c", "abc"}, []string{"--commit", "abc"}}, + {"keeps long", []string{"--format", "json"}, []string{"--format", "json"}}, + {"unknown short kept", []string{"-x", "val"}, []string{"-x", "val"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := expandShortFlags(tc.args, m) + if len(got) != len(tc.want) { + t.Fatalf("got %v, want %v", got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("[%d] = %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} diff --git a/cmd/opencodereview/git_test.go b/cmd/opencodereview/git_test.go new file mode 100644 index 0000000..0630a4c --- /dev/null +++ b/cmd/opencodereview/git_test.go @@ -0,0 +1,153 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func initTestGitRepo(t *testing.T) string { + t.Helper() + dir := t.TempDir() + cmds := [][]string{ + {"git", "-C", dir, "init"}, + {"git", "-C", dir, "config", "user.email", "test@test.com"}, + {"git", "-C", dir, "config", "user.name", "Test"}, + } + for _, args := range cmds { + cmd := exec.Command(args[0], args[1:]...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git init: %v: %s", err, out) + } + } + f := filepath.Join(dir, "README.md") + os.WriteFile(f, []byte("hello"), 0o644) + cmd := exec.Command("git", "-C", dir, "add", ".") + cmd.CombinedOutput() + cmd = exec.Command("git", "-C", dir, "commit", "-m", "initial commit") + cmd.CombinedOutput() + return dir +} + +func TestRunGitCmd_Success(t *testing.T) { + dir := initTestGitRepo(t) + out, err := runGitCmd(dir, "rev-parse", "--git-dir") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(out) == 0 { + t.Error("expected non-empty output") + } +} + +func TestRunGitCmd_Failure(t *testing.T) { + dir := t.TempDir() + _, err := runGitCmd(dir, "rev-parse", "--git-dir") + if err == nil { + t.Error("expected error for non-git dir") + } +} + +func TestGetCommitMessage(t *testing.T) { + dir := initTestGitRepo(t) + msg, err := getCommitMessage(dir, "HEAD") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if msg != "initial commit" { + t.Errorf("msg = %q, want 'initial commit'", msg) + } +} + +func TestGetCommitMessage_InvalidCommit(t *testing.T) { + dir := initTestGitRepo(t) + _, err := getCommitMessage(dir, "nonexistent-ref-xyz") + if err == nil { + t.Fatal("expected error for invalid commit") + } +} + +func TestResolveRepoDir_ValidGitRepo(t *testing.T) { + dir := initTestGitRepo(t) + resolved, err := resolveRepoDir(dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resolved == "" { + t.Error("expected non-empty resolved path") + } +} + +func TestResolveRepoDir_NotGitRepo(t *testing.T) { + dir := t.TempDir() + _, err := resolveRepoDir(dir) + if err == nil { + t.Fatal("expected error for non-git dir") + } + if !strings.Contains(err.Error(), "not a git repository") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestResolveRepoDir_EmptyUsesWd(t *testing.T) { + dir := initTestGitRepo(t) + origDir, _ := os.Getwd() + defer os.Chdir(origDir) + os.Chdir(dir) + + resolved, err := resolveRepoDir("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resolved == "" { + t.Error("expected non-empty resolved path") + } +} + +func TestRequireGitRepo_Valid(t *testing.T) { + dir := initTestGitRepo(t) + if err := requireGitRepo(dir); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRequireGitRepo_Invalid(t *testing.T) { + dir := t.TempDir() + err := requireGitRepo(dir) + if err == nil { + t.Fatal("expected error for non-git dir") + } +} + +func TestValidateReviewRefs_ValidCommit(t *testing.T) { + dir := initTestGitRepo(t) + err := validateReviewRefs(dir, reviewOptions{commit: "HEAD"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestValidateReviewRefs_InvalidCommit(t *testing.T) { + dir := initTestGitRepo(t) + err := validateReviewRefs(dir, reviewOptions{commit: "nonexistent-ref-xyz"}) + if err == nil { + t.Fatal("expected error for invalid commit ref") + } +} + +func TestValidateReviewRefs_EmptySkipped(t *testing.T) { + dir := initTestGitRepo(t) + err := validateReviewRefs(dir, reviewOptions{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestBuildToolRegistry(t *testing.T) { + reg := buildToolRegistry(nil, nil) + if reg == nil { + t.Fatal("expected non-nil registry") + } +} diff --git a/cmd/opencodereview/output_helpers_test.go b/cmd/opencodereview/output_helpers_test.go index 74d8106..d0d25b2 100644 --- a/cmd/opencodereview/output_helpers_test.go +++ b/cmd/opencodereview/output_helpers_test.go @@ -162,6 +162,34 @@ func TestBuildDiffLines(t *testing.T) { }) } +func TestOutputJSONWithWarnings_NoCommentsSubtaskError(t *testing.T) { + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + warnings := []agent.AgentWarning{{Type: "subtask_error", File: "x.go", Message: "fail"}} + err := outputJSONWithWarnings(nil, warnings, 1, 10, 5, 15, 0, 0, time.Second, "", nil) + + w.Close() + os.Stdout = old + + if err != nil { + t.Fatalf("error: %v", err) + } + + var buf bytes.Buffer + buf.ReadFrom(r) + + var out jsonOutput + json.Unmarshal(buf.Bytes(), &out) + if out.Status != "completed_with_errors" { + t.Errorf("status = %q, want completed_with_errors", out.Status) + } + if !strings.Contains(out.Message, "errors") { + t.Errorf("message = %q, expected to mention errors", out.Message) + } +} + func TestStatusBadge(t *testing.T) { tests := []struct { status string @@ -326,3 +354,209 @@ func TestOutputJSONNoFiles(t *testing.T) { t.Errorf("status = %q, want skipped", out.Status) } } + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + os.Stdout = w + fn() + w.Close() + os.Stdout = old + var buf bytes.Buffer + buf.ReadFrom(r) + return buf.String() +} + +func TestOutputText_NoComments(t *testing.T) { + got := captureStdout(t, func() { + outputText(nil) + }) + if !strings.Contains(got, "Looks good to me") { + t.Errorf("expected 'Looks good to me', got %q", got) + } +} + +func TestOutputText_WithComments(t *testing.T) { + comments := []model.LlmComment{ + {Path: "main.go", StartLine: 10, EndLine: 15, Content: "potential nil dereference"}, + } + got := captureStdout(t, func() { + outputText(comments) + }) + if !strings.Contains(got, "main.go") { + t.Errorf("expected path in output, got %q", got) + } + if !strings.Contains(got, "potential nil dereference") { + t.Errorf("expected comment content in output, got %q", got) + } +} + +func TestOutputTextWithWarnings_NoCommentsNoErrors(t *testing.T) { + warnings := []agent.AgentWarning{{Type: "warning", File: "x.go", Message: "slow"}} + got := captureStdout(t, func() { + outputTextWithWarnings(nil, warnings) + }) + if !strings.Contains(got, "Looks good to me") { + t.Errorf("expected 'Looks good to me', got %q", got) + } +} + +func TestOutputTextWithWarnings_NoCommentsWithSubtaskError(t *testing.T) { + warnings := []agent.AgentWarning{{Type: "subtask_error", File: "y.go", Message: "failed"}} + got := captureStdout(t, func() { + outputTextWithWarnings(nil, warnings) + }) + if !strings.Contains(got, "could not be reviewed") { + t.Errorf("expected subtask error message, got %q", got) + } +} + +func TestOutputTextWithWarnings_WithComments(t *testing.T) { + comments := []model.LlmComment{ + {Path: "a.go", StartLine: 1, EndLine: 3, Content: "fix this"}, + } + warnings := []agent.AgentWarning{{Type: "info", File: "b.go", Message: "note"}} + got := captureStdout(t, func() { + outputTextWithWarnings(comments, warnings) + }) + if !strings.Contains(got, "a.go") { + t.Errorf("expected comment path, got %q", got) + } + if !strings.Contains(got, "fix this") { + t.Errorf("expected comment content, got %q", got) + } +} + +func TestRenderComment_EmptyContentNoDiff(t *testing.T) { + got := captureStdout(t, func() { + renderComment(model.LlmComment{Path: "skip.go", StartLine: 1, EndLine: 1, Content: "", ExistingCode: "", SuggestionCode: ""}) + }) + if got != "" { + t.Errorf("expected empty output for empty comment, got %q", got) + } +} + +func TestRenderComment_ContentOnly(t *testing.T) { + got := captureStdout(t, func() { + renderComment(model.LlmComment{Path: "file.go", StartLine: 5, EndLine: 10, Content: "consider renaming"}) + }) + if !strings.Contains(got, "file.go:5-10") { + t.Errorf("expected path:line range, got %q", got) + } + if !strings.Contains(got, "consider renaming") { + t.Errorf("expected content, got %q", got) + } +} + +func TestRenderComment_WithDiff(t *testing.T) { + got := captureStdout(t, func() { + renderComment(model.LlmComment{ + Path: "diff.go", + StartLine: 1, + EndLine: 2, + Content: "rename var", + ExistingCode: "old := 1\n", + SuggestionCode: "new := 1\n", + }) + }) + if !strings.Contains(got, "diff.go:1-2") { + t.Errorf("expected path:line range, got %q", got) + } + if !strings.Contains(got, "rename var") { + t.Errorf("expected content, got %q", got) + } +} + +func TestPrintDiffLine(t *testing.T) { + tests := []struct { + name string + prefix string + content string + }{ + {"added", "+", "new line"}, + {"deleted", "-", "old line"}, + {"context", " ", "context line"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := captureStdout(t, func() { + printDiffLine(tc.prefix, tc.content, "\033[92m", "\033[48;2;0;60;0m") + }) + if !strings.Contains(got, tc.prefix) { + t.Errorf("expected prefix %q in output, got %q", tc.prefix, got) + } + if !strings.Contains(got, tc.content) { + t.Errorf("expected content %q in output, got %q", tc.content, got) + } + }) + } +} + +func TestOutputPreviewText_NoFiles(t *testing.T) { + p := &agent.DiffPreview{TotalFiles: 0} + got := captureStdout(t, func() { + outputPreviewText(p) + }) + if !strings.Contains(got, "No files changed") { + t.Errorf("expected 'No files changed', got %q", got) + } +} + +func TestOutputPreviewText_WithReviewableFiles(t *testing.T) { + p := &agent.DiffPreview{ + Entries: []agent.DiffPreviewEntry{ + {Path: "main.go", Status: "modified", Insertions: 10, Deletions: 3, WillReview: true}, + {Path: "util.go", Status: "added", Insertions: 20, Deletions: 0, WillReview: true}, + }, + TotalInsertions: 30, + TotalDeletions: 3, + TotalFiles: 2, + ReviewableCount: 2, + ExcludedCount: 0, + } + got := captureStdout(t, func() { + outputPreviewText(p) + }) + if !strings.Contains(got, "2 file(s) changed") { + t.Errorf("expected file count, got %q", got) + } + if !strings.Contains(got, "Will review (2)") { + t.Errorf("expected 'Will review' section, got %q", got) + } + if !strings.Contains(got, "main.go") || !strings.Contains(got, "util.go") { + t.Errorf("expected file paths, got %q", got) + } +} + +func TestOutputPreviewText_WithExcludedFiles(t *testing.T) { + p := &agent.DiffPreview{ + Entries: []agent.DiffPreviewEntry{ + {Path: "src.go", Status: "modified", Insertions: 5, Deletions: 1, WillReview: true}, + {Path: "vendor/lib.go", Status: "added", Insertions: 100, Deletions: 0, WillReview: false, ExcludeReason: model.ExcludeDefaultPath}, + }, + TotalInsertions: 105, + TotalDeletions: 1, + TotalFiles: 2, + ReviewableCount: 1, + ExcludedCount: 1, + } + got := captureStdout(t, func() { + outputPreviewText(p) + }) + if !strings.Contains(got, "Will review (1)") { + t.Errorf("expected 'Will review (1)', got %q", got) + } + if !strings.Contains(got, "Excluded from review (1)") { + t.Errorf("expected 'Excluded from review (1)', got %q", got) + } + if !strings.Contains(got, "vendor/lib.go") { + t.Errorf("expected excluded file path, got %q", got) + } + if !strings.Contains(got, "default_path") { + t.Errorf("expected exclude reason, got %q", got) + } +} diff --git a/cmd/opencodereview/provider_cmd_test.go b/cmd/opencodereview/provider_cmd_test.go new file mode 100644 index 0000000..54dc906 --- /dev/null +++ b/cmd/opencodereview/provider_cmd_test.go @@ -0,0 +1,205 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestMaskKey(t *testing.T) { + tests := []struct { + name string + key string + want string + }{ + {"empty", "", "(not set)"}, + {"short", "abcd", "***"}, + {"exactly 8", "12345678", "***"}, + {"normal", "sk-ant-secret-key-1234", "sk-a***1234"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := maskKey(tc.key) + if got != tc.want { + t.Errorf("maskKey(%q) = %q, want %q", tc.key, got, tc.want) + } + }) + } +} + +func TestSaveConfig(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "sub", "config.json") + + cfg := &Config{ + Provider: "anthropic", + Model: "claude-opus-4-6", + Language: "English", + } + + if err := saveConfig(path, cfg); err != nil { + t.Fatalf("saveConfig: %v", err) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("perm = %o, want 600", perm) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + var loaded Config + if err := json.Unmarshal(data, &loaded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if loaded.Provider != "anthropic" { + t.Errorf("Provider = %q", loaded.Provider) + } + if loaded.Language != "English" { + t.Errorf("Language = %q", loaded.Language) + } +} + +func TestApplyProviderDeletions(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + + cfg := &Config{ + Provider: "keep", + CustomProviders: map[string]ProviderEntry{ + "del1": {URL: "https://a.example.com"}, + "del2": {URL: "https://b.example.com"}, + "keep": {URL: "https://c.example.com"}, + }, + } + if err := saveConfig(configPath, cfg); err != nil { + t.Fatalf("saveConfig: %v", err) + } + + clearedActive, err := applyProviderDeletions(configPath, cfg, []string{"del1", "del2"}) + if err != nil { + t.Fatalf("applyProviderDeletions: %v", err) + } + if clearedActive { + t.Error("should not have cleared active provider") + } + if _, exists := cfg.CustomProviders["del1"]; exists { + t.Error("del1 should have been deleted") + } + if _, exists := cfg.CustomProviders["del2"]; exists { + t.Error("del2 should have been deleted") + } + if _, exists := cfg.CustomProviders["keep"]; !exists { + t.Error("keep should still exist") + } +} + +func TestApplyProviderDeletions_ActiveCleared(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + + cfg := &Config{ + Provider: "active-one", + CustomProviders: map[string]ProviderEntry{ + "active-one": {URL: "https://x.example.com"}, + }, + } + if err := saveConfig(configPath, cfg); err != nil { + t.Fatalf("saveConfig: %v", err) + } + + clearedActive, err := applyProviderDeletions(configPath, cfg, []string{"active-one"}) + if err != nil { + t.Fatalf("applyProviderDeletions: %v", err) + } + if !clearedActive { + t.Error("should have cleared active provider") + } +} + +func TestApplyProviderDeletions_SkipsNotFound(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "exists": {URL: "https://a.example.com"}, + }, + } + if err := saveConfig(configPath, cfg); err != nil { + t.Fatalf("saveConfig: %v", err) + } + + _, err := applyProviderDeletions(configPath, cfg, []string{"nonexistent"}) + if err != nil { + t.Fatalf("applyProviderDeletions should not fail, got: %v", err) + } +} + +func TestRemoveModels(t *testing.T) { + tests := []struct { + name string + existing []string + remove []string + want []string + }{ + {"remove one", []string{"a", "b", "c"}, []string{"b"}, []string{"a", "c"}}, + {"remove none", []string{"a", "b"}, []string{"x"}, []string{"a", "b"}}, + {"remove all", []string{"a", "b"}, []string{"a", "b"}, []string{}}, + {"empty existing", nil, []string{"a"}, []string{}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := removeModels(tc.existing, tc.remove) + if len(got) != len(tc.want) { + t.Fatalf("removeModels() = %v, want %v", got, tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("[%d] = %q, want %q", i, got[i], tc.want[i]) + } + } + }) + } +} + +func TestApplyManualConfig_MissingURL(t *testing.T) { + err := applyManualConfig("", &Config{}, providerTUIResult{url: "", model: "m"}) + if err == nil { + t.Fatal("expected error for missing URL") + } +} + +func TestApplyManualConfig_MissingModel(t *testing.T) { + err := applyManualConfig("", &Config{}, providerTUIResult{url: "https://example.com", model: ""}) + if err == nil { + t.Fatal("expected error for missing model") + } +} + +func TestApplyCustomProviderConfig_MissingProvider(t *testing.T) { + err := applyCustomProviderConfig("", &Config{}, providerTUIResult{provider: "", model: "m"}) + if err == nil { + t.Fatal("expected error for missing provider") + } +} + +func TestApplyCustomProviderConfig_MissingModel(t *testing.T) { + err := applyCustomProviderConfig("", &Config{}, providerTUIResult{provider: "p", model: ""}) + if err == nil { + t.Fatal("expected error for missing model") + } +} + +func TestApplyOfficialProviderConfig_MissingFields(t *testing.T) { + err := applyOfficialProviderConfig("", &Config{}, providerTUIResult{provider: "", model: ""}) + if err == nil { + t.Fatal("expected error for missing provider/model") + } +} diff --git a/cmd/opencodereview/provider_tui_funcs_test.go b/cmd/opencodereview/provider_tui_funcs_test.go new file mode 100644 index 0000000..a5f0001 --- /dev/null +++ b/cmd/opencodereview/provider_tui_funcs_test.go @@ -0,0 +1,1160 @@ +package main + +import ( + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/open-code-review/open-code-review/internal/llm" +) + +func TestCustomProviderActiveModel_NilCfg(t *testing.T) { + m := providerTUIModel{existingCfg: nil} + cp := customProviderListItem{name: "test", entry: ProviderEntry{Model: "m1"}} + got := m.customProviderActiveModel(cp) + if got != "" { + t.Errorf("expected empty string for nil cfg, got %q", got) + } +} + +func TestCustomProviderActiveModel_DifferentProvider(t *testing.T) { + cfg := &Config{Provider: "other-provider"} + m := newProviderTUI(cfg, "") + cp := customProviderListItem{name: "test", entry: ProviderEntry{Model: "m1"}} + got := m.customProviderActiveModel(cp) + if got != "" { + t.Errorf("expected empty string for different provider, got %q", got) + } +} + +func TestCustomProviderActiveModel_MatchingProvider(t *testing.T) { + cfg := &Config{ + Provider: "my-custom", + Model: "gpt-4", + CustomProviders: map[string]ProviderEntry{ + "my-custom": {URL: "http://localhost", Model: "gpt-4"}, + }, + } + m := newProviderTUI(cfg, "") + cp := customProviderListItem{name: "my-custom", entry: ProviderEntry{URL: "http://localhost"}} + got := m.customProviderActiveModel(cp) + if got != "gpt-4" { + t.Errorf("expected gpt-4, got %q", got) + } +} + +func TestModelProviderName_OfficialTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + name := m.modelProviderName() + if name == "" { + t.Error("expected non-empty provider name") + } + providers := llm.ListProviders() + if len(providers) == 0 { + t.Skip("no providers registered") + } +} + +func TestModelProviderName_CustomTab(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "my-llm": {URL: "http://localhost", Model: "m"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 0 + name := m.modelProviderName() + if !strings.Contains(name, "(custom)") { + t.Errorf("expected '(custom)' in name, got %q", name) + } +} + +func TestModelProviderName_CustomTab_NoSelection(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.customIdx = 999 + name := m.modelProviderName() + if name != "" { + t.Errorf("expected empty fallback for out-of-bounds custom, got %q", name) + } +} + +func TestModelCount(t *testing.T) { + m := newProviderTUI(&Config{}, "") + count := m.modelCount() + models := m.models() + if count != len(models)+1 { + t.Errorf("modelCount() = %d, want %d", count, len(models)+1) + } +} + +func TestInit_ReturnsNil(t *testing.T) { + m := newProviderTUI(&Config{}, "") + cmd := m.Init() + if cmd != nil { + t.Error("Init() should return nil") + } +} + +func TestListCursorPrefix_Active(t *testing.T) { + got := listCursorPrefix(true) + if !strings.Contains(got, tuiCursor) { + t.Errorf("expected cursor in prefix, got %q", got) + } +} + +func TestListCursorPrefix_Inactive(t *testing.T) { + got := listCursorPrefix(false) + if strings.Contains(got, tuiCursor) { + t.Errorf("expected no cursor in prefix, got %q", got) + } +} + +func TestRenderListName_Active(t *testing.T) { + got := renderListName("my-provider", true) + if !strings.Contains(got, "my-provider") { + t.Errorf("expected name in output, got %q", got) + } +} + +func TestRenderListName_Inactive(t *testing.T) { + got := renderListName("my-provider", false) + if !strings.Contains(got, "my-provider") { + t.Errorf("expected name in output, got %q", got) + } +} + +func TestCloneProviderEntry_WithExtraBody(t *testing.T) { + orig := ProviderEntry{ + APIKey: "key", + URL: "http://localhost", + Protocol: "openai", + Model: "gpt-4", + Models: []string{"gpt-4", "gpt-3.5"}, + AuthHeader: "Authorization", + ExtraBody: map[string]any{"temperature": 0.7, "stream": true}, + } + clone := cloneProviderEntry(orig) + + if clone.APIKey != orig.APIKey || clone.URL != orig.URL || clone.Protocol != orig.Protocol { + t.Error("basic fields not copied") + } + if len(clone.Models) != 2 || clone.Models[0] != "gpt-4" { + t.Errorf("Models not cloned: %v", clone.Models) + } + if clone.ExtraBody == nil { + t.Fatal("ExtraBody should not be nil") + } + if clone.ExtraBody["temperature"] != 0.7 { + t.Errorf("ExtraBody[temperature] = %v", clone.ExtraBody["temperature"]) + } + + clone.ExtraBody["new_key"] = "value" + if _, ok := orig.ExtraBody["new_key"]; ok { + t.Error("modifying clone should not affect original ExtraBody") + } + + clone.Models = append(clone.Models, "gpt-5") + if len(orig.Models) != 2 { + t.Error("modifying clone should not affect original Models") + } +} + +func TestCloneProviderEntry_NilExtraBody(t *testing.T) { + orig := ProviderEntry{ + APIKey: "key", + URL: "http://localhost", + } + clone := cloneProviderEntry(orig) + if clone.ExtraBody != nil { + t.Error("ExtraBody should remain nil") + } +} + +func TestCustomListCount(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "a": {URL: "http://a"}, + "b": {URL: "http://b"}, + }, + } + m := newProviderTUI(cfg, "") + got := m.customListCount() + if got != len(m.customProviders)+1 { + t.Errorf("customListCount() = %d, want %d", got, len(m.customProviders)+1) + } +} + +func TestIsCustomModelItem(t *testing.T) { + m := newProviderTUI(&Config{}, "") + models := m.models() + if m.isCustomModelItem(len(models)) != true { + t.Error("expected true for custom model item index") + } + if len(models) > 0 && m.isCustomModelItem(0) { + t.Error("expected false for non-custom model index") + } +} + +func upKey() tea.KeyPressMsg { + return tea.KeyPressMsg{Code: tea.KeyUp} +} + +func TestHandleUp_OfficialTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + if len(m.providers) < 2 { + t.Skip("need at least 2 providers") + } + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.officialIdx != 1 { + t.Fatalf("after down, officialIdx = %d, want 1", m2.officialIdx) + } + result, _ = m2.Update(upKey()) + m3 := result.(providerTUIModel) + if m3.officialIdx != 0 { + t.Errorf("after up, officialIdx = %d, want 0", m3.officialIdx) + } +} + +func TestHandleUp_Wraps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + if len(m.providers) == 0 { + t.Skip("no providers") + } + result, _ := m.Update(upKey()) + m2 := result.(providerTUIModel) + if m2.officialIdx != len(m2.providers)-1 { + t.Errorf("up from 0 should wrap to %d, got %d", len(m2.providers)-1, m2.officialIdx) + } +} + +func TestHandleUp_CustomTab(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "a": {URL: "http://a"}, + "b": {URL: "http://b"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 1 + result, _ := m.Update(upKey()) + m2 := result.(providerTUIModel) + if m2.customIdx != 0 { + t.Errorf("customIdx = %d, want 0", m2.customIdx) + } +} + +func TestHandleUp_ModelStep(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.modelIdx = 1 + result, _ := m.Update(upKey()) + m2 := result.(providerTUIModel) + if m2.modelIdx != 0 { + t.Errorf("modelIdx = %d, want 0", m2.modelIdx) + } +} + +func TestHandleUp_ModelStepWraps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.modelIdx = 0 + result, _ := m.Update(upKey()) + m2 := result.(providerTUIModel) + expected := m2.modelCount() - 1 + if m2.modelIdx != expected { + t.Errorf("modelIdx = %d, want %d", m2.modelIdx, expected) + } +} + +func TestBlurCPStep_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + for _, step := range []customProviderStep{cpStepName, cpStepBaseURL, cpStepAPIKey, cpStepAuthHeader, cpStepProtocol} { + m.cpStep = step + m.blurCPStep() + } +} + +func TestFocusCPStep_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + for _, step := range []customProviderStep{cpStepName, cpStepBaseURL, cpStepAPIKey, cpStepAuthHeader, cpStepProtocol} { + m.cpStep = step + m.focusCPStep() + } +} + +func TestCollectCustomProviders_Nil(t *testing.T) { + got := collectCustomProviders(nil) + if got != nil { + t.Errorf("expected nil, got %v", got) + } +} + +func TestCollectCustomProviders_NilMap(t *testing.T) { + got := collectCustomProviders(&Config{}) + if got != nil { + t.Errorf("expected nil, got %v", got) + } +} + +func TestCollectCustomProviders_Sorted(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "zebra": {URL: "http://z"}, + "alpha": {URL: "http://a"}, + "mid": {URL: "http://m"}, + }, + } + got := collectCustomProviders(cfg) + if len(got) != 3 { + t.Fatalf("expected 3 items, got %d", len(got)) + } + if got[0].name != "alpha" || got[1].name != "mid" || got[2].name != "zebra" { + t.Errorf("not sorted: %v, %v, %v", got[0].name, got[1].name, got[2].name) + } +} + +func TestCustomProviderNameTaken(t *testing.T) { + m := providerTUIModel{existingCfg: nil} + if m.customProviderNameTaken("test") { + t.Error("nil cfg should return false") + } + + m2 := newProviderTUI(&Config{}, "") + if m2.customProviderNameTaken("test") { + t.Error("nil CustomProviders should return false") + } + + m3 := newProviderTUI(&Config{ + CustomProviders: map[string]ProviderEntry{"test": {URL: "http://test"}}, + }, "") + if !m3.customProviderNameTaken("test") { + t.Error("existing name should return true") + } + if m3.customProviderNameTaken("other") { + t.Error("non-existing name should return false") + } +} + +func TestCurrentProvider_OutOfBounds(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.officialIdx = 9999 + p := m.currentProvider() + if p.Name != "" { + t.Errorf("expected empty provider, got %q", p.Name) + } +} + +func TestCurrentProvider_WrongTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + p := m.currentProvider() + if p.Name != "" { + t.Errorf("expected empty provider for non-official tab, got %q", p.Name) + } +} + +func TestSelectedCustomProvider_NotCustomTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + _, ok := m.selectedCustomProvider() + if ok { + t.Error("expected false for non-custom tab") + } +} + +func TestSelectedCustomProvider_OutOfBounds(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.customIdx = 9999 + _, ok := m.selectedCustomProvider() + if ok { + t.Error("expected false for out-of-bounds index") + } +} + +func TestWindowSizeMsg(t *testing.T) { + m := newProviderTUI(&Config{}, "") + result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40}) + m2 := result.(providerTUIModel) + if m2.width != 120 || m2.height != 40 { + t.Errorf("size = %dx%d, want 120x40", m2.width, m2.height) + } +} + +func TestBlurManualStep_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + for _, step := range []manualStep{manualStepURL, manualStepProtocol, manualStepModel, manualStepAuthToken, manualStepAuthHeader} { + m.manualStep = step + m.blurManualStep() + } +} + +func TestFocusManualStep_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + for _, step := range []manualStep{manualStepURL, manualStepProtocol, manualStepModel, manualStepAuthToken, manualStepAuthHeader} { + m.manualStep = step + m.focusManualStep() + } +} + +func TestHandleDown_OfficialTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + if len(m.providers) < 2 { + t.Skip("need at least 2 providers") + } + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.officialIdx != 1 { + t.Errorf("officialIdx = %d, want 1", m2.officialIdx) + } +} + +func TestHandleDown_OfficialTab_Wraps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + if len(m.providers) == 0 { + t.Skip("no providers") + } + m.officialIdx = len(m.providers) - 1 + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.officialIdx != 0 { + t.Errorf("down from last should wrap to 0, got %d", m2.officialIdx) + } +} + +func TestHandleDown_CustomTab(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "a": {URL: "http://a"}, + "b": {URL: "http://b"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 0 + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.customIdx != 1 { + t.Errorf("customIdx = %d, want 1", m2.customIdx) + } +} + +func TestHandleDown_CustomTab_Wraps(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "a": {URL: "http://a"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = m.customListCount() - 1 + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.customIdx != 0 { + t.Errorf("down from last custom should wrap to 0, got %d", m2.customIdx) + } +} + +func TestHandleDown_ModelStep(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.modelIdx = 0 + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.modelIdx != 1 { + t.Errorf("modelIdx = %d, want 1", m2.modelIdx) + } +} + +func TestHandleDown_ModelStep_Wraps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.modelIdx = m.modelCount() - 1 + result, _ := m.Update(downKey()) + m2 := result.(providerTUIModel) + if m2.modelIdx != 0 { + t.Errorf("modelIdx = %d, want 0", m2.modelIdx) + } +} + +func TestCloneCustomProvidersMap(t *testing.T) { + src := map[string]ProviderEntry{ + "a": {URL: "http://a", ExtraBody: map[string]any{"k": "v"}}, + "b": {URL: "http://b"}, + } + clone := cloneCustomProvidersMap(src) + if len(clone) != 2 { + t.Fatalf("expected 2, got %d", len(clone)) + } + clone["a"] = ProviderEntry{URL: "http://changed"} + if src["a"].URL != "http://a" { + t.Error("modifying clone should not affect original") + } +} + +func TestCloneCustomProvidersMap_Nil(t *testing.T) { + got := cloneCustomProvidersMap(nil) + if got != nil { + t.Errorf("expected nil, got %v", got) + } +} + +func TestCloneCustomProviderList(t *testing.T) { + src := []customProviderListItem{ + {name: "a", entry: ProviderEntry{URL: "http://a"}}, + {name: "b", entry: ProviderEntry{URL: "http://b"}}, + } + clone := cloneCustomProviderList(src) + if len(clone) != 2 { + t.Fatalf("expected 2, got %d", len(clone)) + } + clone[0].name = "changed" + if src[0].name != "a" { + t.Error("modifying clone should not affect original") + } +} + +func TestCustomProviderEntry_FromConfig(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "test": {URL: "http://real", Model: "m1"}, + }, + } + m := newProviderTUI(cfg, "") + fallback := ProviderEntry{URL: "http://fallback"} + got := m.customProviderEntry("test", fallback) + if got.URL != "http://real" { + t.Errorf("expected config entry, got URL %q", got.URL) + } +} + +func TestCustomProviderEntry_Fallback(t *testing.T) { + m := newProviderTUI(&Config{}, "") + fallback := ProviderEntry{URL: "http://fallback"} + got := m.customProviderEntry("nonexist", fallback) + if got.URL != "http://fallback" { + t.Errorf("expected fallback, got URL %q", got.URL) + } +} + +func TestNewModelTUI(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + if m.modelIdx != 0 { + t.Errorf("modelIdx = %d, want 0 for empty currentModel", m.modelIdx) + } + cmd := m.Init() + if cmd != nil { + t.Error("Init() should return nil") + } +} + +func TestNewModelTUI_WithCurrentModel(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 || len(p[0].Models) == 0 { + t.Skip("need provider with models") + } + current := p[0].Models[0] + m := newModelTUI(p[0], current) + if m.modelIdx != 0 { + t.Errorf("modelIdx = %d, want 0 for first model", m.modelIdx) + } + if m.activeModel != current { + t.Errorf("activeModel = %q, want %q", m.activeModel, current) + } +} + +func TestNewModelTUI_CustomModel(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "custom-model-xyz") + if m.modelIdx != len(p[0].Models) { + t.Errorf("modelIdx = %d, want %d for custom model", m.modelIdx, len(p[0].Models)) + } +} + +func TestModelTUI_IsCustomItem(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + if !m.isCustomItem(len(p[0].Models)) { + t.Error("expected true for custom item index") + } + if m.isCustomItem(0) { + t.Error("expected false for index 0") + } +} + +func TestModelTUI_ItemCount(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + if m.itemCount() != len(p[0].Models)+1 { + t.Errorf("itemCount() = %d, want %d", m.itemCount(), len(p[0].Models)+1) + } +} + +func TestModelTUI_SelectedModel(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 || len(p[0].Models) == 0 { + t.Skip("need provider with models") + } + m := newModelTUI(p[0], "") + got := m.selectedModel() + if got != p[0].Models[0] { + t.Errorf("selectedModel() = %q, want %q", got, p[0].Models[0]) + } +} + +func TestModelTUI_SelectedModel_OutOfBounds(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + m.modelIdx = 9999 + got := m.selectedModel() + if got != "" { + t.Errorf("expected empty for out-of-bounds, got %q", got) + } +} + +func TestModelTUI_Update_UpDown(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 || len(p[0].Models) < 2 { + t.Skip("need provider with at least 2 models") + } + m := newModelTUI(p[0], "") + result, _ := m.Update(downKey()) + m2 := result.(modelTUIModel) + if m2.modelIdx != 1 { + t.Errorf("after down, modelIdx = %d, want 1", m2.modelIdx) + } + result, _ = m2.Update(upKey()) + m3 := result.(modelTUIModel) + if m3.modelIdx != 0 { + t.Errorf("after up, modelIdx = %d, want 0", m3.modelIdx) + } +} + +func TestModelTUI_Update_WindowSize(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + result, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 50}) + m2 := result.(modelTUIModel) + if m2.width != 100 || m2.height != 50 { + t.Errorf("size = %dx%d, want 100x50", m2.width, m2.height) + } +} + +func TestModelTUI_Update_EscCancels(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + result, _ := m.Update(escKey()) + m2 := result.(modelTUIModel) + if !m2.cancelled { + t.Error("expected cancelled after esc") + } +} + +// --- View rendering tests (smoke) --- + +func TestProviderTUIView_StepProvider_OfficialTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + v := m.View() + if v.Content == "" { + t.Error("expected non-empty view") + } +} + +func TestProviderTUIView_StepProvider_CustomTab(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "my-llm": {URL: "http://localhost", Model: "m"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + v := m.View() + if !strings.Contains(v.Content, "my-llm") { + t.Errorf("expected custom provider name in view, got %q", v.Content) + } +} + +func TestProviderTUIView_StepProvider_CustomTab_CreatingCustom(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.creatingCustom = true + v := m.View() + if !strings.Contains(v.Content, "Add Custom Provider") { + t.Errorf("expected 'Add Custom Provider' in view") + } +} + +func TestProviderTUIView_StepProvider_CustomTab_EditingCustom(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "ed": {URL: "http://ed"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.editingCustom = true + m.editTargetName = "ed" + v := m.View() + if !strings.Contains(v.Content, "Edit Custom Provider") { + t.Errorf("expected 'Edit Custom Provider' in view") + } +} + +func TestProviderTUIView_StepProvider_ManualTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + v := m.View() + if !strings.Contains(v.Content, "Manual") { + t.Errorf("expected 'Manual' in view") + } +} + +func TestProviderTUIView_StepProvider_ManualTab_InForm(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + m.inManualForm = true + v := m.View() + if !strings.Contains(v.Content, "Manual Configuration") { + t.Errorf("expected 'Manual Configuration' in view") + } +} + +func TestProviderTUIView_StepProvider_ConfirmingDelete(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "del": {URL: "http://del"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.confirmingDelete = true + m.deleteTargetName = "del" + v := m.View() + if !strings.Contains(v.Content, "Confirm") { + t.Errorf("expected confirm help text in view") + } +} + +func TestProviderTUIView_StepModel(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + v := m.View() + if !strings.Contains(v.Content, "Select a model") { + t.Errorf("expected 'Select a model' in view, got %q", v.Content) + } +} + +func TestProviderTUIView_StepModel_CustomModel(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.customModel = true + v := m.View() + if v.Content == "" { + t.Error("expected non-empty view") + } +} + +func TestProviderTUIView_StepModel_FormError(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.customModel = true + m.formError = "model name required" + v := m.View() + if !strings.Contains(v.Content, "model name required") { + t.Errorf("expected form error in view") + } +} + +func TestProviderTUIView_StepModel_ConfirmingDeleteModel(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepModel + m.confirmingDeleteModel = true + m.deleteModelName = "gpt-4" + v := m.View() + if !strings.Contains(v.Content, "gpt-4") { + t.Errorf("expected delete model name in view") + } +} + +func TestProviderTUIView_StepAPIKey(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.step = stepAPIKey + v := m.View() + if !strings.Contains(v.Content, "API Key") { + t.Errorf("expected 'API Key' in view, got %q", v.Content) + } +} + +func TestProviderTUIView_StepAPIKey_CustomProvider(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "cp": {URL: "http://cp"}, + }, + } + m := newProviderTUI(cfg, "") + m.step = stepAPIKey + m.activeTab = tabCustom + m.customIdx = 0 + v := m.View() + if !strings.Contains(v.Content, "cp") { + t.Errorf("expected custom provider name in API Key view") + } +} + +func TestRenderTabBar_AllTabs(t *testing.T) { + for _, tab := range []providerTab{tabOfficial, tabCustom, tabManual} { + got := renderTabBar(tab) + if got == "" { + t.Errorf("renderTabBar(%d) returned empty", tab) + } + } +} + +func TestModelTUI_View(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + v := m.View() + if !strings.Contains(v.Content, "Select a model") { + t.Errorf("expected 'Select a model' in view, got %q", v.Content) + } +} + +func TestModelTUI_View_CustomModel(t *testing.T) { + p := llm.ListProviders() + if len(p) == 0 { + t.Skip("no providers") + } + m := newModelTUI(p[0], "") + m.customModel = true + v := m.View() + if v.Content == "" { + t.Error("expected non-empty view") + } +} + +// --- result() tests --- + +func TestResult_OfficialTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + r := m.result() + if r.provider == "" && len(m.providers) > 0 { + t.Error("expected non-empty provider") + } +} + +func TestResult_OfficialTab_WithMaskedKey(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.apiKeyMasked = true + m.apiKeyOriginal = "sk-secret" + r := m.result() + if r.apiKey != "sk-secret" { + t.Errorf("expected masked key, got %q", r.apiKey) + } +} + +func TestResult_CustomTab_Creating(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.creatingCustom = true + m.cpNameInput.SetValue("new-prov") + m.cpURLInput.SetValue("http://url") + r := m.result() + if !r.isCustom { + t.Error("expected isCustom=true") + } + if r.provider != "new-prov" { + t.Errorf("provider = %q, want new-prov", r.provider) + } +} + +func TestResult_CustomTab_Editing(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "ed": {URL: "http://ed", Model: "m1", Models: []string{"m1", "m2"}}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.editingCustom = true + m.editTargetName = "ed" + m.cpNameInput.SetValue("ed") + m.cpURLInput.SetValue("http://ed") + r := m.result() + if !r.isEdit { + t.Error("expected isEdit=true") + } + if r.model != "m1" { + t.Errorf("model = %q, want m1", r.model) + } +} + +func TestResult_CustomTab_Selected(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "sel": {URL: "http://sel", Model: "gpt", Models: []string{"gpt"}}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 0 + r := m.result() + if !r.isCustom { + t.Error("expected isCustom=true") + } + if r.provider != "sel" { + t.Errorf("provider = %q, want sel", r.provider) + } +} + +func TestResult_CustomTab_OutOfBounds(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.customIdx = 999 + r := m.result() + if r.provider != "" { + t.Errorf("expected empty provider, got %q", r.provider) + } +} + +func TestResult_ManualTab(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + m.manualURLInput.SetValue("http://manual") + m.manualModelInput.SetValue("model-x") + r := m.result() + if !r.isManual { + t.Error("expected isManual=true") + } + if r.url != "http://manual" { + t.Errorf("url = %q, want http://manual", r.url) + } + if r.model != "model-x" { + t.Errorf("model = %q, want model-x", r.model) + } +} + +func TestResult_ManualTab_MaskedToken(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + m.manualTokenMasked = true + m.manualTokenOriginal = "tok-secret" + r := m.result() + if r.apiKey != "tok-secret" { + t.Errorf("expected masked token, got %q", r.apiKey) + } +} + +// --- loadExistingAPIKey tests --- + +func TestLoadExistingAPIKey_CustomTab_HasKey(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "cp": {URL: "http://cp", APIKey: "sk-key"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 0 + m.loadExistingAPIKey() + if !m.apiKeyMasked { + t.Error("expected apiKeyMasked=true") + } + if m.apiKeyOriginal != "sk-key" { + t.Errorf("apiKeyOriginal = %q, want sk-key", m.apiKeyOriginal) + } +} + +func TestLoadExistingAPIKey_CustomTab_NoKey(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "cp": {URL: "http://cp"}, + }, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabCustom + m.customIdx = 0 + m.loadExistingAPIKey() + if m.apiKeyMasked { + t.Error("expected apiKeyMasked=false") + } +} + +func TestLoadExistingAPIKey_OfficialTab_NilCfg(t *testing.T) { + m := providerTUIModel{existingCfg: nil} + m.loadExistingAPIKey() + if m.apiKeyMasked { + t.Error("expected apiKeyMasked=false for nil cfg") + } +} + +func TestLoadExistingAPIKey_OfficialTab_HasKey(t *testing.T) { + cfg := &Config{} + m := newProviderTUI(cfg, "") + if len(m.providers) == 0 { + t.Skip("no providers") + } + provName := m.providers[0].Name + cfg.Providers = map[string]ProviderEntry{ + provName: {APIKey: "official-key"}, + } + m.existingCfg = cfg + m.officialIdx = 0 + m.loadExistingAPIKey() + if !m.apiKeyMasked { + t.Error("expected apiKeyMasked=true") + } + if m.apiKeyOriginal != "official-key" { + t.Errorf("apiKeyOriginal = %q, want official-key", m.apiKeyOriginal) + } +} + +// --- selectedModelFromState tests --- + +func TestSelectedModelFromState_CustomModelInput(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.customModel = true + m.modelInput.SetValue("my-model") + got := m.selectedModelFromState() + if got != "my-model" { + t.Errorf("expected my-model, got %q", got) + } +} + +func TestSelectedModelFromState_OutOfBounds(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.modelIdx = 9999 + got := m.selectedModelFromState() + if got != "" { + t.Errorf("expected empty, got %q", got) + } +} + +// --- syncSessionModelSelection tests --- + +func TestSyncSessionModelSelection_NilCfg(t *testing.T) { + m := providerTUIModel{existingCfg: nil} + err := m.syncSessionModelSelection() + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +func TestSyncSessionModelSelection_EmptyModel(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.modelIdx = 9999 + err := m.syncSessionModelSelection() + if err != nil { + t.Errorf("unexpected error: %v", err) + } +} + +// --- viewCustomProviderForm field steps --- + +func TestProviderTUIView_CustomForm_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.creatingCustom = true + for _, step := range []customProviderStep{cpStepName, cpStepBaseURL, cpStepAPIKey, cpStepAuthHeader, cpStepProtocol} { + m.cpStep = step + v := m.View() + if v.Content == "" { + t.Errorf("empty view for step %d", step) + } + } +} + +func TestProviderTUIView_CustomForm_WithError(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabCustom + m.creatingCustom = true + m.formError = "name is required" + v := m.View() + if !strings.Contains(v.Content, "name is required") { + t.Error("expected form error in view") + } +} + +// --- viewManualTab field steps --- + +func TestProviderTUIView_ManualForm_AllSteps(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + m.inManualForm = true + for _, step := range []manualStep{manualStepURL, manualStepProtocol, manualStepModel, manualStepAuthToken, manualStepAuthHeader} { + m.manualStep = step + v := m.View() + if v.Content == "" { + t.Errorf("empty view for manual step %d", step) + } + } +} + +func TestProviderTUIView_ManualForm_WithError(t *testing.T) { + m := newProviderTUI(&Config{}, "") + m.activeTab = tabManual + m.inManualForm = true + m.formError = "URL required" + v := m.View() + if !strings.Contains(v.Content, "URL required") { + t.Error("expected form error in view") + } +} + +func TestProviderTUIView_ManualTab_WithExistingConfig(t *testing.T) { + cfg := &Config{ + Llm: LlmConfig{URL: "http://existing", Model: "old-model"}, + } + m := newProviderTUI(cfg, "") + m.activeTab = tabManual + v := m.View() + if !strings.Contains(v.Content, "http://existing") { + t.Errorf("expected existing URL in manual tab") + } +} + +// --- viewModel with custom tab and deleteModel --- + +func TestProviderTUIView_StepModel_CustomTabDeleteHelp(t *testing.T) { + cfg := &Config{ + CustomProviders: map[string]ProviderEntry{ + "cp": {URL: "http://cp", Models: []string{"m1"}}, + }, + } + m := newProviderTUI(cfg, "") + m.step = stepModel + m.activeTab = tabCustom + m.customIdx = 0 + v := m.View() + if !strings.Contains(v.Content, "Delete") { + t.Errorf("expected delete help for custom tab model view") + } +} diff --git a/cmd/opencodereview/scan_cmd_test.go b/cmd/opencodereview/scan_cmd_test.go index c20e20a..0205842 100644 --- a/cmd/opencodereview/scan_cmd_test.go +++ b/cmd/opencodereview/scan_cmd_test.go @@ -140,3 +140,110 @@ func TestParseScanFlags_HelpFlag(t *testing.T) { t.Error("opts.showHelp should be true when -h is supplied") } } + +func TestParseScanFlags_RejectsNegativeMaxTokensBudget(t *testing.T) { + _, err := parseScanFlags([]string{"--max-tokens-budget", "-100"}) + if err == nil { + t.Fatal("expected error for negative --max-tokens-budget") + } + if !strings.Contains(err.Error(), "--max-tokens-budget") { + t.Errorf("error message = %q; want it to mention --max-tokens-budget", err.Error()) + } +} + +func TestParseScanFlags_BooleanFlags(t *testing.T) { + opts, err := parseScanFlags([]string{"--no-plan", "--no-dedup", "--no-summary", "--preview"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.noPlan { + t.Error("noPlan should be true") + } + if !opts.noDedup { + t.Error("noDedup should be true") + } + if !opts.noSummary { + t.Error("noSummary should be true") + } + if !opts.preview { + t.Error("preview should be true") + } +} + +func TestParseScanFlags_ModelOverride(t *testing.T) { + opts, err := parseScanFlags([]string{"--model", "claude-opus-4-6"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.model != "claude-opus-4-6" { + t.Errorf("model = %q, want claude-opus-4-6", opts.model) + } +} + +func TestParseScanFlags_AllStringFlags(t *testing.T) { + opts, err := parseScanFlags([]string{ + "--tools", "/tmp/tools.json", + "--rule", "/tmp/rule.json", + "--repo", "/tmp/repo", + "--exclude", "*.md,*.txt", + "--batch", "by-language", + "--background", "test context", + "--audience", "agent", + "-f", "json", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.toolConfigPath != "/tmp/tools.json" { + t.Errorf("toolConfigPath = %q", opts.toolConfigPath) + } + if opts.rulePath != "/tmp/rule.json" { + t.Errorf("rulePath = %q", opts.rulePath) + } + if opts.repoDir != "/tmp/repo" { + t.Errorf("repoDir = %q", opts.repoDir) + } + if opts.excludes != "*.md,*.txt" { + t.Errorf("excludes = %q", opts.excludes) + } + if opts.batch != "by-language" { + t.Errorf("batch = %q", opts.batch) + } + if opts.background != "test context" { + t.Errorf("background = %q", opts.background) + } + if opts.audience != "agent" { + t.Errorf("audience = %q", opts.audience) + } + if opts.outputFormat != "json" { + t.Errorf("outputFormat = %q", opts.outputFormat) + } +} + +func TestParseScanFlags_IntFlags(t *testing.T) { + opts, err := parseScanFlags([]string{ + "--concurrency", "16", + "--timeout", "20", + "--max-tools", "50", + "--max-git-procs", "32", + "--max-tokens-budget", "100000", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.concurrency != 16 { + t.Errorf("concurrency = %d", opts.concurrency) + } + if opts.perFileTimeout != 20 { + t.Errorf("perFileTimeout = %d", opts.perFileTimeout) + } + if opts.maxTools != 50 { + t.Errorf("maxTools = %d", opts.maxTools) + } + if opts.maxGitProcs != 32 { + t.Errorf("maxGitProcs = %d", opts.maxGitProcs) + } + if opts.maxTokensBudget != 100000 { + t.Errorf("maxTokensBudget = %d", opts.maxTokensBudget) + } +} diff --git a/cmd/opencodereview/shared_test.go b/cmd/opencodereview/shared_test.go new file mode 100644 index 0000000..8c9b819 --- /dev/null +++ b/cmd/opencodereview/shared_test.go @@ -0,0 +1,127 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/open-code-review/open-code-review/internal/config/rules" +) + +func TestApplyCLIExcludes_Empty(t *testing.T) { + cc := &commonContext{FileFilter: &rules.FileFilter{Exclude: []string{"a"}}} + applyCLIExcludes(cc, nil) + if len(cc.FileFilter.Exclude) != 1 { + t.Errorf("expected 1 exclude, got %d", len(cc.FileFilter.Exclude)) + } +} + +func TestApplyCLIExcludes_AppendsPatterns(t *testing.T) { + cc := &commonContext{FileFilter: &rules.FileFilter{Exclude: []string{"a"}}} + applyCLIExcludes(cc, []string{"b", "c"}) + if len(cc.FileFilter.Exclude) != 3 { + t.Errorf("expected 3 excludes, got %d", len(cc.FileFilter.Exclude)) + } +} + +func TestApplyCLIExcludes_NilFileFilter(t *testing.T) { + cc := &commonContext{} + applyCLIExcludes(cc, []string{"x"}) + if cc.FileFilter == nil { + t.Fatal("expected FileFilter to be created") + } + if len(cc.FileFilter.Exclude) != 1 || cc.FileFilter.Exclude[0] != "x" { + t.Errorf("expected [x], got %v", cc.FileFilter.Exclude) + } +} + +func TestNewQuietHandle_NoOp(t *testing.T) { + h := newQuietHandle("text", "developer") + if h.fn != nil { + t.Error("expected no-op handle for text/developer") + } + h.Restore() +} + +func TestNewQuietHandle_JSON(t *testing.T) { + h := newQuietHandle("json", "developer") + if h.fn == nil { + t.Error("expected fn to be set for json format") + } + h.Restore() + if h.fn != nil { + t.Error("expected fn to be nil after Restore") + } +} + +func TestNewQuietHandle_Agent(t *testing.T) { + h := newQuietHandle("text", "agent") + if h.fn == nil { + t.Error("expected fn to be set for agent audience") + } + h.Restore() +} + +func TestQuietHandle_NilReceiver(t *testing.T) { + var h *quietHandle + h.Restore() +} + +func TestQuietHandle_IdempotentRestore(t *testing.T) { + h := newQuietHandle("json", "developer") + h.Restore() + h.Restore() + if h.fn != nil { + t.Error("expected nil after double restore") + } +} + +func TestResolveWorkingDir_CurrentDir(t *testing.T) { + dir := t.TempDir() + origDir, _ := os.Getwd() + defer os.Chdir(origDir) + os.Chdir(dir) + + absPath, isGit, err := resolveWorkingDir("", false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if absPath == "" { + t.Error("expected non-empty absPath") + } + if isGit { + t.Error("temp dir should not be a git repo") + } +} + +func TestResolveWorkingDir_RequireGitFails(t *testing.T) { + dir := t.TempDir() + _, _, err := resolveWorkingDir(dir, true) + if err == nil { + t.Fatal("expected error for non-git dir with requireGit=true") + } +} + +func TestResolveWorkingDir_NonExistent(t *testing.T) { + _, _, err := resolveWorkingDir(filepath.Join(t.TempDir(), "no-such-dir"), false) + if err == nil { + t.Fatal("expected error for non-existent path") + } +} + +func TestResolveWorkingDir_GitRepo(t *testing.T) { + dir := t.TempDir() + gitDir := filepath.Join(dir, ".git") + if err := os.Mkdir(gitDir, 0o755); err != nil { + t.Fatal(err) + } + + absPath, isGit, err := resolveWorkingDir(dir, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if absPath == "" { + t.Error("expected non-empty absPath") + } + _ = isGit +} diff --git a/cmd/opencodereview/smallfiles_test.go b/cmd/opencodereview/smallfiles_test.go new file mode 100644 index 0000000..cae63f2 --- /dev/null +++ b/cmd/opencodereview/smallfiles_test.go @@ -0,0 +1,248 @@ +package main + +import ( + "runtime" + "strings" + "testing" +) + +func TestPrintVersion_Dev(t *testing.T) { + origVersion := Version + origCommit := GitCommit + origDate := BuildDate + defer func() { + Version = origVersion + GitCommit = origCommit + BuildDate = origDate + }() + + Version = "dev" + GitCommit = "" + BuildDate = "" + + got := captureStdout(t, func() { + printVersion() + }) + if !strings.Contains(got, "open-code-review dev") { + t.Errorf("expected 'open-code-review dev', got %q", got) + } + if !strings.Contains(got, runtime.GOOS+"/"+runtime.GOARCH) { + t.Errorf("expected OS/ARCH, got %q", got) + } +} + +func TestPrintVersion_WithCommitAndDate(t *testing.T) { + origVersion := Version + origCommit := GitCommit + origDate := BuildDate + defer func() { + Version = origVersion + GitCommit = origCommit + BuildDate = origDate + }() + + Version = "1.2.3" + GitCommit = "abc1234" + BuildDate = "2026-01-01" + + got := captureStdout(t, func() { + printVersion() + }) + if !strings.Contains(got, "1.2.3") { + t.Errorf("expected version, got %q", got) + } + if !strings.Contains(got, "abc1234") { + t.Errorf("expected commit, got %q", got) + } + if !strings.Contains(got, "2026-01-01") { + t.Errorf("expected build date, got %q", got) + } +} + +func TestParseViewerFlags_Defaults(t *testing.T) { + opts, err := parseViewerFlags(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.addr != "localhost:5483" { + t.Errorf("addr = %q, want localhost:5483", opts.addr) + } + if opts.showHelp { + t.Error("showHelp should be false") + } +} + +func TestParseViewerFlags_CustomAddr(t *testing.T) { + opts, err := parseViewerFlags([]string{"--addr", ":3000"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if opts.addr != ":3000" { + t.Errorf("addr = %q, want :3000", opts.addr) + } +} + +func TestParseViewerFlags_Help(t *testing.T) { + opts, err := parseViewerFlags([]string{"-h"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !opts.showHelp { + t.Error("expected showHelp=true") + } +} + +func TestRunLLM_NoArgs(t *testing.T) { + got := captureStdout(t, func() { + err := runLLM(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "LLM utility") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunLLM_UnknownSubcommand(t *testing.T) { + err := runLLM([]string{"bogus"}) + if err == nil { + t.Fatal("expected error for unknown subcommand") + } + if !strings.Contains(err.Error(), "unknown llm sub-command") { + t.Errorf("unexpected error: %v", err) + } +} + +func TestRunRules_NoArgs(t *testing.T) { + got := captureStdout(t, func() { + err := runRules(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "ocr rules") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunRules_Help(t *testing.T) { + got := captureStdout(t, func() { + err := runRules([]string{"-h"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "ocr rules") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunRules_UnknownSubcommand(t *testing.T) { + err := runRules([]string{"bogus"}) + if err == nil { + t.Fatal("expected error for unknown subcommand") + } +} + +func TestRunRules_HelpAltFlag(t *testing.T) { + got := captureStdout(t, func() { + err := runRules([]string{"--help"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "ocr rules") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunRulesCheck_Help(t *testing.T) { + got := captureStdout(t, func() { + err := runRulesCheck([]string{"-h"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "ocr rules check") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunRulesCheck_NoArgs(t *testing.T) { + got := captureStdout(t, func() { + err := runRulesCheck(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "ocr rules check") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestRunLLMProviders(t *testing.T) { + got := captureStdout(t, func() { + runLLMProviders() + }) + if !strings.Contains(got, "Built-in providers") { + t.Errorf("expected provider listing, got %q", got) + } +} + +func TestRunViewer_Help(t *testing.T) { + got := captureStdout(t, func() { + err := runViewer([]string{"-h"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + if !strings.Contains(got, "Session history") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestPrintReviewUsage(t *testing.T) { + got := captureStdout(t, func() { + printReviewUsage() + }) + if !strings.Contains(got, "ocr review") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestPrintTopLevelUsage(t *testing.T) { + got := captureStdout(t, func() { + printTopLevelUsage() + }) + if !strings.Contains(got, "OpenCodeReview") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestPrintViewerUsage(t *testing.T) { + got := captureStdout(t, func() { + printViewerUsage() + }) + if !strings.Contains(got, "Session history") { + t.Errorf("expected viewer usage text, got %q", got) + } +} + +func TestPrintRulesCheckUsage(t *testing.T) { + got := captureStdout(t, func() { + printRulesCheckUsage() + }) + if !strings.Contains(got, "ocr rules check") { + t.Errorf("expected usage text, got %q", got) + } +} + +func TestPrintScanUsage(t *testing.T) { + got := captureStdout(t, func() { + printScanUsage() + }) + if !strings.Contains(got, "ocr scan") { + t.Errorf("expected usage text, got %q", got) + } +}