From 9a11fc1302a3b1ef48db4ab95e65bc9c04df565f Mon Sep 17 00:00:00 2001 From: kite Date: Sat, 27 Jun 2026 01:47:48 +0800 Subject: [PATCH] test: improve coverage for cmd/opencodereview package from 42% to 70% Add comprehensive unit tests covering config dispatch, emit run result, git helpers, provider commands, provider TUI pure functions and View rendering, shared utilities, flags parsing, scan command flags, and small file entry points (version, viewer, llm, rules commands). --- cmd/opencodereview/config_cmd_test.go | 374 ++++++ cmd/opencodereview/config_dispatch_test.go | 30 + cmd/opencodereview/emit_run_result_test.go | 177 +++ cmd/opencodereview/flags_test.go | 176 ++- cmd/opencodereview/git_test.go | 153 +++ cmd/opencodereview/output_helpers_test.go | 234 ++++ cmd/opencodereview/provider_cmd_test.go | 205 +++ cmd/opencodereview/provider_tui_funcs_test.go | 1160 +++++++++++++++++ cmd/opencodereview/scan_cmd_test.go | 107 ++ cmd/opencodereview/shared_test.go | 127 ++ cmd/opencodereview/smallfiles_test.go | 248 ++++ 11 files changed, 2990 insertions(+), 1 deletion(-) create mode 100644 cmd/opencodereview/config_dispatch_test.go create mode 100644 cmd/opencodereview/emit_run_result_test.go create mode 100644 cmd/opencodereview/git_test.go create mode 100644 cmd/opencodereview/provider_cmd_test.go create mode 100644 cmd/opencodereview/provider_tui_funcs_test.go create mode 100644 cmd/opencodereview/shared_test.go create mode 100644 cmd/opencodereview/smallfiles_test.go 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) + } +}