From ba8beb53b3b3c9b9e67f562e58e0b095559a377f Mon Sep 17 00:00:00 2001 From: "Charles P. Cross" Date: Thu, 18 Dec 2025 17:42:45 -0500 Subject: [PATCH] test(coverage): add tests to meet 45% CI threshold Add comprehensive test coverage for previously untested functions: internal/debug/debug_test.go: - TestSetVerbose: tests SetVerbose() and its effect on Enabled() - TestSetQuietAndIsQuiet: tests SetQuiet() and IsQuiet() functions - TestPrintNormal: tests quiet mode suppression of normal output - TestPrintlnNormal: tests quiet mode suppression of println output internal/export/config_test.go (new file): - TestLoadConfig: comprehensive tests for LoadConfig() including: - Default values when no config exists - Loading custom policies (both regular and auto-export) - Loading retry attempts, backoff, skip encoding errors, write manifest - Handling invalid/malformed config values gracefully internal/export/policy_test.go: - TestErrorPolicyString: tests String() method on ErrorPolicy - TestNewManifest: tests manifest creation with proper defaults - TestWriteManifest: tests manifest file writing and error handling These tests bring coverage from 44.8% to 45.0%, meeting the CI threshold. --- internal/debug/debug_test.go | 146 +++++++++++++++++++++ internal/export/config_test.go | 231 +++++++++++++++++++++++++++++++++ internal/export/policy_test.go | 67 ++++++++++ 3 files changed, 444 insertions(+) create mode 100644 internal/export/config_test.go diff --git a/internal/debug/debug_test.go b/internal/debug/debug_test.go index e49aafde..e4fc17d9 100644 --- a/internal/debug/debug_test.go +++ b/internal/debug/debug_test.go @@ -137,3 +137,149 @@ func TestPrintf(t *testing.T) { }) } } + +func TestSetVerbose(t *testing.T) { + oldVerbose := verboseMode + oldEnabled := enabled + defer func() { + verboseMode = oldVerbose + enabled = oldEnabled + }() + + enabled = false + verboseMode = false + + if Enabled() { + t.Error("Enabled() should be false initially") + } + + SetVerbose(true) + if !Enabled() { + t.Error("Enabled() should be true after SetVerbose(true)") + } + + SetVerbose(false) + if Enabled() { + t.Error("Enabled() should be false after SetVerbose(false)") + } +} + +func TestSetQuietAndIsQuiet(t *testing.T) { + oldQuiet := quietMode + defer func() { quietMode = oldQuiet }() + + quietMode = false + + if IsQuiet() { + t.Error("IsQuiet() should be false initially") + } + + SetQuiet(true) + if !IsQuiet() { + t.Error("IsQuiet() should be true after SetQuiet(true)") + } + + SetQuiet(false) + if IsQuiet() { + t.Error("IsQuiet() should be false after SetQuiet(false)") + } +} + +func TestPrintNormal(t *testing.T) { + tests := []struct { + name string + quiet bool + format string + args []interface{} + wantOutput string + }{ + { + name: "outputs when not quiet", + quiet: false, + format: "info: %s\n", + args: []interface{}{"message"}, + wantOutput: "info: message\n", + }, + { + name: "no output when quiet", + quiet: true, + format: "info: %s\n", + args: []interface{}{"message"}, + wantOutput: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldQuiet := quietMode + oldStdout := os.Stdout + defer func() { + quietMode = oldQuiet + os.Stdout = oldStdout + }() + + quietMode = tt.quiet + + r, w, _ := os.Pipe() + os.Stdout = w + + PrintNormal(tt.format, tt.args...) + + w.Close() + var buf bytes.Buffer + io.Copy(&buf, r) + + if got := buf.String(); got != tt.wantOutput { + t.Errorf("PrintNormal() output = %q, want %q", got, tt.wantOutput) + } + }) + } +} + +func TestPrintlnNormal(t *testing.T) { + tests := []struct { + name string + quiet bool + args []interface{} + wantOutput string + }{ + { + name: "outputs when not quiet", + quiet: false, + args: []interface{}{"hello", "world"}, + wantOutput: "hello world\n", + }, + { + name: "no output when quiet", + quiet: true, + args: []interface{}{"hello", "world"}, + wantOutput: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + oldQuiet := quietMode + oldStdout := os.Stdout + defer func() { + quietMode = oldQuiet + os.Stdout = oldStdout + }() + + quietMode = tt.quiet + + r, w, _ := os.Pipe() + os.Stdout = w + + PrintlnNormal(tt.args...) + + w.Close() + var buf bytes.Buffer + io.Copy(&buf, r) + + if got := buf.String(); got != tt.wantOutput { + t.Errorf("PrintlnNormal() output = %q, want %q", got, tt.wantOutput) + } + }) + } +} diff --git a/internal/export/config_test.go b/internal/export/config_test.go new file mode 100644 index 00000000..7732bf5d --- /dev/null +++ b/internal/export/config_test.go @@ -0,0 +1,231 @@ +package export + +import ( + "context" + "testing" +) + +// mockConfigStore implements ConfigStore for testing +type mockConfigStore struct { + configs map[string]string + err error +} + +func newMockConfigStore() *mockConfigStore { + return &mockConfigStore{ + configs: make(map[string]string), + } +} + +func (m *mockConfigStore) GetConfig(ctx context.Context, key string) (string, error) { + if m.err != nil { + return "", m.err + } + return m.configs[key], nil +} + +func (m *mockConfigStore) SetConfig(ctx context.Context, key, value string) error { + if m.err != nil { + return m.err + } + m.configs[key] = value + return nil +} + +func TestLoadConfig(t *testing.T) { + ctx := context.Background() + + t.Run("returns defaults when no config", func(t *testing.T) { + store := newMockConfigStore() + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Policy != DefaultErrorPolicy { + t.Errorf("Policy = %v, want %v", cfg.Policy, DefaultErrorPolicy) + } + if cfg.RetryAttempts != DefaultRetryAttempts { + t.Errorf("RetryAttempts = %v, want %v", cfg.RetryAttempts, DefaultRetryAttempts) + } + if cfg.RetryBackoffMS != DefaultRetryBackoffMS { + t.Errorf("RetryBackoffMS = %v, want %v", cfg.RetryBackoffMS, DefaultRetryBackoffMS) + } + if cfg.SkipEncodingErrors != DefaultSkipEncodingErrors { + t.Errorf("SkipEncodingErrors = %v, want %v", cfg.SkipEncodingErrors, DefaultSkipEncodingErrors) + } + if cfg.WriteManifest != DefaultWriteManifest { + t.Errorf("WriteManifest = %v, want %v", cfg.WriteManifest, DefaultWriteManifest) + } + }) + + t.Run("loads custom policy", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyErrorPolicy] = string(PolicyBestEffort) + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Policy != PolicyBestEffort { + t.Errorf("Policy = %v, want %v", cfg.Policy, PolicyBestEffort) + } + }) + + t.Run("loads auto-export policy when isAutoExport", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyAutoExportPolicy] = string(PolicyPartial) + cfg, err := LoadConfig(ctx, store, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Policy != PolicyPartial { + t.Errorf("Policy = %v, want %v", cfg.Policy, PolicyPartial) + } + if !cfg.IsAutoExport { + t.Error("IsAutoExport should be true") + } + }) + + t.Run("loads retry attempts", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryAttempts] = "5" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryAttempts != 5 { + t.Errorf("RetryAttempts = %v, want 5", cfg.RetryAttempts) + } + }) + + t.Run("loads retry backoff", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryBackoffMS] = "200" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryBackoffMS != 200 { + t.Errorf("RetryBackoffMS = %v, want 200", cfg.RetryBackoffMS) + } + }) + + t.Run("loads skip encoding errors", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeySkipEncodingErrors] = "true" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cfg.SkipEncodingErrors { + t.Error("SkipEncodingErrors should be true") + } + }) + + t.Run("loads write manifest", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyWriteManifest] = "true" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cfg.WriteManifest { + t.Error("WriteManifest should be true") + } + }) + + t.Run("ignores invalid policy", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyErrorPolicy] = "invalid-policy" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Policy != DefaultErrorPolicy { + t.Errorf("Policy = %v, want %v (default)", cfg.Policy, DefaultErrorPolicy) + } + }) + + t.Run("ignores invalid retry attempts", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryAttempts] = "not-a-number" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryAttempts != DefaultRetryAttempts { + t.Errorf("RetryAttempts = %v, want %v (default)", cfg.RetryAttempts, DefaultRetryAttempts) + } + }) + + t.Run("ignores negative retry attempts", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryAttempts] = "-1" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryAttempts != DefaultRetryAttempts { + t.Errorf("RetryAttempts = %v, want %v (default)", cfg.RetryAttempts, DefaultRetryAttempts) + } + }) + + t.Run("ignores invalid retry backoff", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryBackoffMS] = "not-a-number" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryBackoffMS != DefaultRetryBackoffMS { + t.Errorf("RetryBackoffMS = %v, want %v (default)", cfg.RetryBackoffMS, DefaultRetryBackoffMS) + } + }) + + t.Run("ignores zero retry backoff", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyRetryBackoffMS] = "0" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.RetryBackoffMS != DefaultRetryBackoffMS { + t.Errorf("RetryBackoffMS = %v, want %v (default)", cfg.RetryBackoffMS, DefaultRetryBackoffMS) + } + }) + + t.Run("ignores invalid skip encoding errors", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeySkipEncodingErrors] = "not-a-bool" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.SkipEncodingErrors != DefaultSkipEncodingErrors { + t.Errorf("SkipEncodingErrors = %v, want %v (default)", cfg.SkipEncodingErrors, DefaultSkipEncodingErrors) + } + }) + + t.Run("ignores invalid write manifest", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyWriteManifest] = "not-a-bool" + cfg, err := LoadConfig(ctx, store, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.WriteManifest != DefaultWriteManifest { + t.Errorf("WriteManifest = %v, want %v (default)", cfg.WriteManifest, DefaultWriteManifest) + } + }) + + t.Run("falls back to general policy if auto-export not set", func(t *testing.T) { + store := newMockConfigStore() + store.configs[ConfigKeyErrorPolicy] = string(PolicyBestEffort) + cfg, err := LoadConfig(ctx, store, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Policy != PolicyBestEffort { + t.Errorf("Policy = %v, want %v", cfg.Policy, PolicyBestEffort) + } + }) +} diff --git a/internal/export/policy_test.go b/internal/export/policy_test.go index 4787ad93..107cbdea 100644 --- a/internal/export/policy_test.go +++ b/internal/export/policy_test.go @@ -3,6 +3,7 @@ package export import ( "context" "errors" + "os" "testing" "time" ) @@ -97,6 +98,72 @@ func TestErrorPolicy(t *testing.T) { } } +func TestErrorPolicyString(t *testing.T) { + tests := []struct { + policy ErrorPolicy + want string + }{ + {PolicyStrict, "strict"}, + {PolicyBestEffort, "best-effort"}, + {PolicyPartial, "partial"}, + {PolicyRequiredCore, "required-core"}, + } + + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + if got := tt.policy.String(); got != tt.want { + t.Errorf("String() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestNewManifest(t *testing.T) { + manifest := NewManifest(PolicyBestEffort) + + if manifest == nil { + t.Fatal("NewManifest returned nil") + } + if manifest.ErrorPolicy != string(PolicyBestEffort) { + t.Errorf("ErrorPolicy = %v, want %v", manifest.ErrorPolicy, PolicyBestEffort) + } + if !manifest.Complete { + t.Error("Complete should be true by default") + } + if manifest.ExportedAt.IsZero() { + t.Error("ExportedAt should not be zero") + } +} + +func TestWriteManifest(t *testing.T) { + t.Run("writes manifest successfully", func(t *testing.T) { + tmpDir := t.TempDir() + jsonlPath := tmpDir + "/test.jsonl" + + manifest := NewManifest(PolicyStrict) + manifest.ExportedCount = 5 + manifest.Complete = true + + err := WriteManifest(jsonlPath, manifest) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify manifest file was created + manifestPath := tmpDir + "/test.manifest.json" + if _, err := os.Stat(manifestPath); os.IsNotExist(err) { + t.Error("manifest file was not created") + } + }) + + t.Run("fails on invalid directory", func(t *testing.T) { + err := WriteManifest("/nonexistent/path/test.jsonl", NewManifest(PolicyStrict)) + if err == nil { + t.Error("expected error for nonexistent directory") + } + }) +} + func TestFetchWithPolicy(t *testing.T) { ctx := context.Background()