diff --git a/CLAUDE.md b/CLAUDE.md index 3091873..900f2c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,7 +5,7 @@ **Never say a feature is complete without running the actual binary.** ```bash -cd /Users/steve.calvert/workspace/personal/glean-cli +cd /Users/steve.calvert/workspace/glean/glean-cli mise run build # build the binary ./glean # run the TUI and test the feature manually ``` @@ -56,11 +56,26 @@ mise run test:all # lint + test + build (CI equivalent) ## TUI Architecture Rules -- **Worktree LSP errors are noise** — the worktree (`/.claude/worktrees/`) is stale. Always build in the main repo at `/Users/steve.calvert/workspace/personal/glean-cli` +- **Worktree LSP errors are noise** — the worktree (`/.claude/worktrees/`) is stale. Always build in the main repo at `/Users/steve.calvert/workspace/glean/glean-cli` - **Viewport key isolation** — never pass `tea.KeyMsg` to viewport in the catch-all; scroll keys are handled explicitly above - **Collect-then-display for content** — streaming stages show live via channel; content waits until complete - **No viewport jumping** — `conversationActive` pins viewport height; `recalculateLayout()` only called on terminal resize or deliberate state changes +## Auth Test Isolation + +**All tests that touch auth, config, or keyring state MUST call `authtest.IsolateAuthState(t)`.** + +```go +import "github.com/gleanwork/glean-cli/internal/auth/authtest" + +func TestSomethingWithAuth(t *testing.T) { + authtest.IsolateAuthState(t) + // ... test code +} +``` + +This redirects HOME, config path, and keyring to a temp directory so tests never read or delete real credentials. Without it, tests can silently wipe `~/.glean/config.json` and keyring entries. + ## Common Mistakes to Avoid 1. **Rendering picker only in active state** — UI elements visible before a conversation starts need to work in the welcome state too (`!m.conversationActive` branch in View()) diff --git a/cmd/auth_test.go b/cmd/auth_test.go index b82b3c3..b2e9d6b 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -4,6 +4,7 @@ import ( "bytes" "testing" + "github.com/gleanwork/glean-cli/internal/auth/authtest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,28 +32,25 @@ func TestAuthCmd_HasSubcommands(t *testing.T) { } func TestAuthStatusCmd_NoConfig(t *testing.T) { - // With no config set, auth status should not panic. - // It prints to stdout directly (not cmd.OutOrStdout), so we just - // verify it doesn't return an error. + authtest.IsolateAuthState(t) + root := NewCmdRoot() buf := &bytes.Buffer{} root.SetOut(buf) root.SetErr(buf) root.SetArgs([]string{"auth", "status"}) - // Should not crash — returns nil (prints "Not configured.") or a wrapped error. _ = root.Execute() } func TestAuthLogoutCmd_NoPanic(t *testing.T) { - // Verify logout doesn't panic regardless of system auth state. + authtest.IsolateAuthState(t) + root := NewCmdRoot() buf := &bytes.Buffer{} root.SetOut(buf) root.SetErr(buf) root.SetArgs([]string{"auth", "logout"}) - // May succeed or fail depending on whether credentials exist on - // this machine — either outcome is fine, we just verify no panic. _ = root.Execute() } diff --git a/internal/auth/auth_persistence_test.go b/internal/auth/auth_persistence_test.go index 0b36597..21f2000 100644 --- a/internal/auth/auth_persistence_test.go +++ b/internal/auth/auth_persistence_test.go @@ -2,35 +2,17 @@ package auth_test import ( "context" - "path/filepath" "testing" "time" "github.com/gleanwork/glean-cli/internal/auth" + "github.com/gleanwork/glean-cli/internal/auth/authtest" gleanClient "github.com/gleanwork/glean-cli/internal/client" "github.com/gleanwork/glean-cli/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/zalando/go-keyring" ) -func isolateAuthState(t *testing.T) { - t.Helper() - - home := t.TempDir() - t.Setenv("HOME", home) - - oldConfigPath := config.ConfigPath - config.ConfigPath = filepath.Join(home, ".glean", "config.json") - t.Cleanup(func() { config.ConfigPath = oldConfigPath }) - - oldServiceName := config.ServiceName - config.ServiceName = "glean-cli-test-auth-persistence" - t.Cleanup(func() { config.ServiceName = oldServiceName }) - - keyring.MockInit() -} - func oauthToken() *auth.StoredTokens { return &auth.StoredTokens{ AccessToken: "oauth-access-token", @@ -43,7 +25,7 @@ func oauthToken() *auth.StoredTokens { } func TestOAuthLoginStateRequiresPersistedHostAfterEnvHostIsRemoved(t *testing.T) { - isolateAuthState(t) + authtest.IsolateAuthState(t) const host = "acme-be.glean.com" require.NoError(t, auth.SaveTokens(host, oauthToken())) @@ -68,7 +50,7 @@ func TestOAuthLoginStateRequiresPersistedHostAfterEnvHostIsRemoved(t *testing.T) } func TestOAuthTokenResolvesWhenHostIsPersisted(t *testing.T) { - isolateAuthState(t) + authtest.IsolateAuthState(t) const host = "acme-be.glean.com" require.NoError(t, config.SaveHostToFile(host)) @@ -84,7 +66,7 @@ func TestOAuthTokenResolvesWhenHostIsPersisted(t *testing.T) { } func TestShortFormHostNormalizesConsistently(t *testing.T) { - isolateAuthState(t) + authtest.IsolateAuthState(t) const shortHost = "acme" const normalizedHost = "acme-be.glean.com" @@ -107,7 +89,7 @@ func TestShortFormHostNormalizesConsistently(t *testing.T) { } func TestLogoutClearsPersistedHostAndOAuthTokens(t *testing.T) { - isolateAuthState(t) + authtest.IsolateAuthState(t) const host = "acme-be.glean.com" require.NoError(t, config.SaveHostToFile(host)) diff --git a/internal/auth/authtest/authtest.go b/internal/auth/authtest/authtest.go new file mode 100644 index 0000000..4371390 --- /dev/null +++ b/internal/auth/authtest/authtest.go @@ -0,0 +1,30 @@ +// Package authtest provides shared test helpers for auth-related tests. +package authtest + +import ( + "path/filepath" + "testing" + + "github.com/gleanwork/glean-cli/internal/config" + "github.com/zalando/go-keyring" +) + +// IsolateAuthState redirects config, HOME, and keyring to temporary +// locations so tests never touch real credentials. All mutations are +// reverted via t.Cleanup. +func IsolateAuthState(t *testing.T) { + t.Helper() + + home := t.TempDir() + t.Setenv("HOME", home) + + oldConfigPath := config.ConfigPath + config.ConfigPath = filepath.Join(home, ".glean", "config.json") + t.Cleanup(func() { config.ConfigPath = oldConfigPath }) + + oldServiceName := config.ServiceName + config.ServiceName = "glean-cli-test-isolated" + t.Cleanup(func() { config.ServiceName = oldServiceName }) + + keyring.MockInit() +}