Skip to content

Improve local test reliability and output#1154

Open
khaong wants to merge 6 commits intomainfrom
codex/local-testing-improvements
Open

Improve local test reliability and output#1154
khaong wants to merge 6 commits intomainfrom
codex/local-testing-improvements

Conversation

@khaong
Copy link
Copy Markdown
Contributor

@khaong khaong commented May 8, 2026

https://entire.io/gh/entireio/cli/trails/328

Summary

  • switch local test tasks to gotestsum formats that avoid garbled output and show integration test progress
  • prevent integration subprocesses from hanging on inherited TTY prompts
  • isolate login integration auth storage from the local OS keyring
  • make the activity grouping test stable across local time zones and clean up test prompt spew

Test Plan

  • go test -tags=integration ./cmd/entire/cli/integration_test/... -run '^(TestEnableDefaultStrategy|TestLogin_SavesTokenAfterApproval)$' -count=1 -v
  • gotestsum --format testname --format-icons text --hide-summary skipped -- -tags=integration ./cmd/entire/cli/integration_test/... -count=1
  • mise run test:integration
  • mise run test
  • mise run lint

Note

Medium Risk
Moderate risk because it touches authentication token persistence logic (adding an alternate file-backed path), though it’s gated behind a test-only env var; most other changes are test/task reliability tweaks.

Overview
Improves local and integration test reliability by forcing CLI subprocesses to run detached from the parent TTY (via execx.NonInteractive) so they can’t hang on interactive prompts.

Adds an optional file-backed auth token store controlled by ENTIRE_TEST_AUTH_STORE_FILE, letting integration login tests persist tokens across subprocesses without using the OS keyring (with secure 0700/0600 perms).

Stabilizes and de-flakes tests and output: activity commit grouping tests now use local-time timestamps, the agent multipicker exits early on cancelled contexts, TUI sink tests actively dismiss the UI, the GitHub repo-name prompt writes to the provided output, and mise test tasks switch to gotestsum for cleaner progress/reporting.

Reviewed by Cursor Bugbot for commit ac3c014. Configure here.

Entire-Checkpoint: 7a9ca45bcbe4
Copilot AI review requested due to automatic review settings May 8, 2026 11:05
@khaong khaong requested a review from a team as a code owner May 8, 2026 11:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves local and integration test reliability for the Entire CLI by making test output clearer, avoiding hangs from inherited TTY behavior, and isolating integration auth storage from the OS keyring.

Changes:

  • Switch local mise test tasks to gotestsum formats for cleaner output and integration test progress visibility.
  • Detach integration subprocesses from the parent TTY and route auth token persistence to a file-backed store for tests.
  • Stabilize a day-grouping test across time zones and reduce TUI test flakiness by ensuring the TUI is dismissed.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
mise.toml Updates local test tasks to use gotestsum for clearer output.
cmd/entire/cli/setup_github.go Routes a repository-name prompt’s output to the provided writer to reduce prompt spew/TTY coupling.
cmd/entire/cli/review/tui_sink_test.go Adds a helper to finish and dismiss the Bubble Tea TUI to prevent hangs in tests.
cmd/entire/cli/integration_test/setup_cmd_test.go Runs entire enable via a non-interactive subprocess to avoid inherited TTY prompts.
cmd/entire/cli/integration_test/login_test.go Sets an env var so integration login stores tokens in an isolated file instead of the OS keyring.
cmd/entire/cli/auth/store.go Adds optional file-backed token store (enabled via env var) for subprocess/integration tests.
cmd/entire/cli/auth/store_test.go Adds coverage for the file-backed auth store behavior and file permissions.
cmd/entire/cli/activity_cmd_test.go Makes commit-day grouping test stable across local time zones by using local timestamps.
Comments suppressed due to low confidence (1)

cmd/entire/cli/auth/store.go:38

  • NewStoreWithService() no longer initializes testStoreFile from ENTIRE_TEST_AUTH_STORE_FILE, so setting that env var will still route SaveToken/GetToken/DeleteToken through the OS keyring for stores created via NewStoreWithService. Consider reading the env var here as well (or refactoring into a shared constructor) so all Store instances consistently honor the file-backed test store option.
// NewStoreWithService returns a Store with a custom keyring service name (for testing).
func NewStoreWithService(service string) *Store {
	return &Store{service: service}
}

Comment thread cmd/entire/cli/auth/store.go
Comment thread cmd/entire/cli/review/tui_sink_test.go
Entire-Checkpoint: 556f162ba317
@khaong
Copy link
Copy Markdown
Contributor Author

khaong commented May 8, 2026

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit ac3c014. Configure here.

khaong and others added 4 commits May 8, 2026 21:25
Entire-Checkpoint: 8b543a5d28eb
Splits the file-backed auth token storage out of the always-compiled
production code and into a build-tagged file (//go:build authfilestore),
so production CLI builds physically cannot opt into reading
ENTIRE_TEST_AUTH_STORE_FILE — closing the footgun where a stray env
var would silently bypass the OS keyring.

- store.go: refactor to a small tokenBackend interface with keyringBackend
  as the always-compiled default; chooseBackend is the only seam.
- store_filebackend.go: //go:build authfilestore — owns the env-var read,
  the file backend impl, and an init() that swaps chooseBackend.
- store_filebackend_test.go: tagged unit test for the file backend.
- store_invariants_test.go: untagged guard test that scans the auth
  package and fails if forbidden symbols (the env var name, os.WriteFile,
  os.ReadFile) appear outside a //go:build authfilestore file. Catches
  drift on every test run.
- setup_test.go: build the integration-test subprocess with
  -tags=authfilestore so it can use the file backend.
- mise.toml: pass -tags=integration,authfilestore in test:integration
  and test:ci, and include ./cmd/entire/cli/auth/... in test:integration
  so the tagged file-backend test runs.

Verified: untagged "go build ./..." produces a binary with zero
references to ENTIRE_TEST_AUTH_STORE_FILE / fileBackend / "test auth
store" (checked via strings | grep). Tagged build retains them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: ea20ee1b0608
NewTUISink constructed tea.NewProgram without tea.WithInput, so Bubble
Tea defaulted to os.Stdin — and when the test binary inherits a real
terminal, that means it called term.MakeRaw on the user's TTY. With
three parallel TUI tests, the restore-on-quit path raced and one of
them returned while the terminal was still in raw mode, leaving OPOST
disabled. Subsequent gotestsum output then rendered with stair-stepped
line breaks (\n moves the cursor down without returning it to col 0).

Add an io.Reader parameter to NewTUISink. Production callers pass
os.Stdin (unchanged behavior — the TUI path only runs when isTTY &&
canPrompt). Tests pass bytes.NewReader(nil); since that isn't an
*os.File, Bubble Tea skips raw-mode setup entirely, so the parallel
TUI tests can no longer corrupt sibling test output.

Verified: go test -race -count=3 ./cmd/entire/cli/review/... stable
across runs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: b7c4fac87aa9
@khaong khaong enabled auto-merge May 9, 2026 01:22
Comment thread cmd/entire/cli/auth/store.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants