From 8de54d74a79246416d0823fbf11825cae181afe2 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 22 May 2026 00:42:56 +0530 Subject: [PATCH] test: raise coverage to 78% (target: 95) Adds 3 test files covering: - internal/tokens: Load (absent + present), Add+Find+Remove+Save roundtrip, FindByTypeNameEnv (case-insensitive type+name, empty-env-as-development fallback, empty-type guard), normalize() helper, auto-CreatedAt stamping, Save bad-path error, storePath() HOME resolution - internal/secretstore: UseDefault non-clobbering of existing backend, UseDefault with disabled keychain returns nil, keychainBackend Available() env-disable path, Name() label, MemoryBackend Set("") deletes - cmd: shortToken truncate logic, extractTagged tag-scan (request_id, upgrade, end-of-string, newline-terminator, absent-tag), humanMessage preference order (message > agent_action > code > empty) Coverage: 69.0% -> 77.7%. Remaining gap is cmd/login.go pollForTierUpgrade + runUpgrade (require live API), cmd/root.go Execute + main.go (CLI entrypoint glue), and secretstore/secretstore.go Get/Set/Delete against the real OS keychain (intentionally excluded by INSTANT_DISABLE_KEYCHAIN in tests to avoid touching the dev keychain). Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/coverage_helpers_test.go | 63 ++++++++++ internal/secretstore/keychain_test.go | 77 ++++++++++++ internal/tokens/store_test.go | 161 ++++++++++++++++++++++++++ 3 files changed, 301 insertions(+) create mode 100644 cmd/coverage_helpers_test.go create mode 100644 internal/secretstore/keychain_test.go create mode 100644 internal/tokens/store_test.go diff --git a/cmd/coverage_helpers_test.go b/cmd/coverage_helpers_test.go new file mode 100644 index 0000000..b956ea8 --- /dev/null +++ b/cmd/coverage_helpers_test.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "testing" +) + +// TestShortToken covers the 0%-line in up.go. +func TestShortToken(t *testing.T) { + cases := map[string]string{ + "": "", + "abc": "abc", + "abcdefgh": "abcdefgh", + "abcdefghi": "abcdefgh", + "verylongtoken": "verylong", + } + for in, want := range cases { + got := shortToken(in) + if got != want { + t.Errorf("shortToken(%q) = %q, want %q", in, got, want) + } + } +} + +// TestExtractTagged covers the partial-coverage helper in json_error.go. +func TestExtractTagged(t *testing.T) { + s := "blah request_id=abc-123 other_tag=xx upgrade=https://foo trailing" + if got := extractTagged(s, "request_id="); got != "abc-123" { + t.Errorf("extractTagged request_id = %q", got) + } + if got := extractTagged(s, "upgrade="); got != "https://foo" { + t.Errorf("extractTagged upgrade = %q", got) + } + // Tag at end of string runs to EOS. + if got := extractTagged("x trailing=end", "trailing="); got != "end" { + t.Errorf("trailing tag = %q", got) + } + // Missing tag returns "". + if got := extractTagged(s, "absent="); got != "" { + t.Errorf("missing tag should yield empty, got %q", got) + } + // Stop on newline. + if got := extractTagged("v=stop\nmore", "v="); got != "stop" { + t.Errorf("newline should terminate, got %q", got) + } +} + +// TestHumanMessage covers apierror.go humanMessage preference order. +func TestHumanMessage(t *testing.T) { + cases := []struct { + env apiErrorEnvelope + want string + }{ + {apiErrorEnvelope{Message: "msg", AgentAction: "act", Error: "code"}, "msg"}, + {apiErrorEnvelope{AgentAction: "act", Error: "code"}, "act"}, + {apiErrorEnvelope{Error: "code"}, "code"}, + {apiErrorEnvelope{}, ""}, + } + for _, c := range cases { + if got := c.env.humanMessage(); got != c.want { + t.Errorf("humanMessage(%+v) = %q, want %q", c.env, got, c.want) + } + } +} diff --git a/internal/secretstore/keychain_test.go b/internal/secretstore/keychain_test.go new file mode 100644 index 0000000..9a621a6 --- /dev/null +++ b/internal/secretstore/keychain_test.go @@ -0,0 +1,77 @@ +package secretstore + +import ( + "testing" +) + +// TestUseDefault_LeavesExistingBackend confirms UseDefault is non-clobbering. +func TestUseDefault_LeavesExistingBackend(t *testing.T) { + m := UseMemoryBackend() + defer Use(nil) + + if got := UseDefault(); got != m { + t.Errorf("UseDefault should preserve in-memory backend, got %T", got) + } +} + +// TestUseDefault_NoBackend_DisabledKeychain — when the env var disables the +// keychain and no backend is active, UseDefault returns nil. +func TestUseDefault_DisabledKeychain(t *testing.T) { + Use(nil) + t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1") + got := UseDefault() + if got != nil { + t.Errorf("UseDefault with disabled keychain should return nil, got %T", got) + } +} + +// TestKeychainBackend_Available_DisabledByEnv verifies the env override. +func TestKeychainBackend_Available_DisabledByEnv(t *testing.T) { + t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1") + k := &keychainBackend{} + if k.Available() { + t.Error("keychain should report unavailable when INSTANT_DISABLE_KEYCHAIN=1") + } + if k.Name() != "keychain" { + t.Errorf("Name() = %q", k.Name()) + } +} + +// TestKeychainBackend_SetEmptyDeletes covers the "Set("") -> Delete" branch +// even when the keychain itself is unavailable (calling Delete is idempotent +// and safe to invoke against a missing/unavailable backend). +func TestKeychainBackend_SetEmptyDeletes(t *testing.T) { + t.Setenv("INSTANT_DISABLE_KEYCHAIN", "1") + k := &keychainBackend{} + // Set("") routes to Delete(); Delete must be idempotent, so this should + // either return nil or a non-fatal error from go-keyring on hosts without + // a keychain. Just ensure it doesn't panic. + _ = k.Set("") +} + +// TestMemoryBackend_Direct exercises *MemoryBackend methods through the type. +func TestMemoryBackend_Direct(t *testing.T) { + m := &MemoryBackend{} + if _, err := m.Get(); err != ErrNotFound { + t.Errorf("empty Get should return ErrNotFound, got %v", err) + } + if err := m.Set("x"); err != nil { + t.Fatalf("Set: %v", err) + } + if v, _ := m.Get(); v != "x" { + t.Errorf("Get = %q", v) + } + if err := m.Set(""); err != nil { + t.Fatalf("Set empty: %v", err) + } + if _, err := m.Get(); err != ErrNotFound { + t.Error("Set(\"\") should clear") + } + if !m.Available() { + t.Error("MemoryBackend always available") + } + if m.Name() != "memory" { + t.Errorf("Name = %q", m.Name()) + } + _ = m.Delete() +} diff --git a/internal/tokens/store_test.go b/internal/tokens/store_test.go new file mode 100644 index 0000000..7b66ff4 --- /dev/null +++ b/internal/tokens/store_test.go @@ -0,0 +1,161 @@ +package tokens + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// setupTempHome redirects HOME to a temp dir for the test, ensuring +// storePath() lands in an isolated location. +func setupTempHome(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + return dir +} + +func TestLoad_AbsentFile(t *testing.T) { + setupTempHome(t) + s, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(s.Entries) != 0 { + t.Errorf("expected empty entries, got %d", len(s.Entries)) + } +} + +func TestAdd_Find_Remove_Save(t *testing.T) { + dir := setupTempHome(t) + s, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + + if err := s.Add(Entry{Token: "tok1", Name: "db1", Type: "postgres", URL: "postgres://x"}); err != nil { + t.Fatalf("Add: %v", err) + } + if err := s.Add(Entry{Token: "tok2", Name: "c1", Type: "redis", URL: "redis://x", CreatedAt: time.Now().Add(-time.Hour)}); err != nil { + t.Fatalf("Add: %v", err) + } + + // Find roundtrips + if e := s.Find("tok1"); e == nil || e.Name != "db1" { + t.Errorf("Find(tok1) = %+v", e) + } + if e := s.Find("missing"); e != nil { + t.Errorf("Find(missing) = %+v, want nil", e) + } + + // File written + if _, err := os.Stat(filepath.Join(dir, ".instant-tokens")); err != nil { + t.Errorf("expected file written, got %v", err) + } + + // Reload preserves data + s2, err := Load() + if err != nil { + t.Fatalf("Load2: %v", err) + } + if len(s2.Entries) != 2 { + t.Errorf("expected 2 entries, got %d", len(s2.Entries)) + } + + // Remove existing & missing + if !s2.Remove("tok1") { + t.Error("Remove(existing) should return true") + } + if s2.Remove("nope") { + t.Error("Remove(missing) should return false") + } + if s2.Find("tok1") != nil { + t.Error("tok1 should be gone") + } +} + +func TestFindByTypeNameEnv(t *testing.T) { + setupTempHome(t) + s, _ := Load() + _ = s.Add(Entry{Token: "tok1", Name: "DB One", Type: "Postgres", Env: "production"}) + _ = s.Add(Entry{Token: "tok2", Name: "cache", Type: "redis", Env: ""}) // legacy empty env + _ = s.Add(Entry{Token: "tok3", Name: "x", Type: ""}) // legacy empty type — ignored + + // Case-insensitive type+name match. + if e := s.FindByTypeNameEnv("postgres", "db one", "production"); e == nil || e.Token != "tok1" { + t.Errorf("expected tok1, got %+v", e) + } + // Empty cached env matches default "development". + if e := s.FindByTypeNameEnv("redis", "cache", "development"); e == nil || e.Token != "tok2" { + t.Errorf("expected tok2, got %+v", e) + } + // Empty cached env also matches empty. + if e := s.FindByTypeNameEnv("redis", "cache", ""); e == nil { + t.Errorf("empty env should match") + } + // Different env → no match. + if e := s.FindByTypeNameEnv("postgres", "db one", "staging"); e != nil { + t.Errorf("staging should not match production entry, got %+v", e) + } + // Empty type entry is never returned. + if e := s.FindByTypeNameEnv("", "x", ""); e != nil { + t.Errorf("empty-type entry returned: %+v", e) + } + // Unknown combo. + if e := s.FindByTypeNameEnv("kafka", "nope", "dev"); e != nil { + t.Errorf("expected nil for unknown, got %+v", e) + } +} + +func TestNormalize_Internal(t *testing.T) { + cases := map[string]string{ + "": "", + " Foo ": "foo", + "a\tB\nC": "abc", + "POSTGRES": "postgres", + } + for in, want := range cases { + got := normalize(in) + if got != want { + t.Errorf("normalize(%q) = %q, want %q", in, got, want) + } + } +} + +func TestAdd_AutoTimestamps(t *testing.T) { + setupTempHome(t) + s, _ := Load() + before := time.Now().UTC() + if err := s.Add(Entry{Token: "auto1"}); err != nil { + t.Fatalf("Add: %v", err) + } + e := s.Find("auto1") + if e == nil { + t.Fatal("Find returned nil") + } + if e.CreatedAt.IsZero() { + t.Error("CreatedAt should be auto-populated") + } + if e.CreatedAt.Before(before.Add(-time.Second)) { + t.Errorf("CreatedAt %v predates test start %v", e.CreatedAt, before) + } +} + +func TestStorePath_Internal(t *testing.T) { + dir := setupTempHome(t) + p, err := storePath() + if err != nil { + t.Fatalf("storePath: %v", err) + } + if p != filepath.Join(dir, ".instant-tokens") { + t.Errorf("storePath = %q, want %q", p, filepath.Join(dir, ".instant-tokens")) + } +} + +func TestSave_BadPath(t *testing.T) { + s := &Store{path: "/nonexistent-directory/no/such/file"} + if err := s.Save(); err == nil { + t.Error("expected error writing to bogus path") + } +}