test: extract shared test helpers and fixtures#81
Merged
Conversation
Consolidate the per-file `lines(spy)` console-capture readers and the repeated `process.exit` throwing-spy into src/_fixtures/testing.ts, so command and output tests share one implementation instead of redefining them in five files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The proxy env-var snapshot/clear/restore scaffolding was duplicated verbatim in the fetch-with-retry and http-dispatcher transport tests. Move it to src/_fixtures/proxy-env.ts so both suites share one implementation and can't drift on teardown. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add STORED_USER_ADA (the snake-case StoredUser twin of STORED_ACCOUNT) to the shared auth fixtures and consume it in user-records tests, whose list() assertions already require the two shapes to stay in lockstep. Collapse the twice-repeated stored-account list literal in the account tests into a local STORED_LIST const. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Route the apiRequest tests through the shared okResponse/errResponse helpers (matching how the migrate-auth and auth-provider suites already mock fetchWithRetry), keeping only the deliberate malformed-JSON case inline. In fetch-with-retry, replace the repeated `new Response(...)` literals with okResponse and extract the duplicated abort-on-signal fetch implementation into a single abortableFetch helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Engineering/Product document and collection pairs were duplicated verbatim across several resolve-ref cases. Hoist them to local named consts so the shared example data lives in one place while each test keeps its query and assertion inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The command-surface tests stubbed ../lib/auth.js with an identical logged-in config-file user. Extract that into mockOutlineAuthModule so the two suites (and future command tests) share one stub. Skip sharing the config.js/spinner vi.mock blocks: they hook vitest's hoisting, differ per file, and a wrapper would trade clarity for almost no line savings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
reviewed
May 24, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR provides a great cleanup of the test suite by extracting shared helpers, environment fixtures, and API responses to significantly reduce duplication. Consolidating these setup steps and mocks will make future test authoring smoother and keep the test architecture cleaner. There are just a few adjustments needed before merging, including aligning the exported auth mock shapes with the actual implementation, resolving hidden cross-suite state introduced by the global proxy environment snapshot, and deriving the new Ada user fixture directly from the account object to prevent future drift.
- Align mockOutlineAuthModule with the real auth.ts exports (getActiveTokenSource, drop the non-existent getTokenSource/clearConfig) and accept overrides; reuse it in auth-command tests. - Derive STORED_USER_ADA from STORED_ACCOUNT so the two can't drift. - Make the proxy-env snapshot per-suite (captureProxyEnv + restoreProxyEnv(snapshot)) to remove hidden cross-suite state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit of the test suite for duplicated setup, mocks, and entity literals, followed by a tiered cleanup. Each tier is a self-contained commit. Net: 14 files, +180/-244 lines; all 221 tests still pass.
src/_fixtures/testing.ts):lines/linesText(read console-spy calls),mockProcessExit, andmockOutlineAuthModule(default auth stub for command-surface tests) — previously redefined across 5+ files.src/_fixtures/proxy-env.ts): the snapshot/clear/restore scaffolding was duplicated verbatim in the two transport tests.STORED_USER_ADA(snake-caseStoredUsertwin ofSTORED_ACCOUNT) whose mapping the user-records tests already assert; collapsed the repeated account-list literal into a localSTORED_LIST.apiRequesttests through the sharedokResponse/errResponse(matching how migrate-auth/auth-provider already mockfetchWithRetry); extracted the duplicated abort-on-signal fetch mock into oneabortableFetch.What was deliberately left alone
Several audit findings were textual similarity rather than true duplication, and extracting them would have hurt readability or coupled unrelated tests:
vi.mockblocks (vitest hoisting + per-file differences).expectCliError,parseJsonOutput, etc.) — wrapping native vitest matchers degrades failure diagnostics for ~0 savings.Test plan
npm run test— 23 files, 221 tests passnpm run type-checkclean🤖 Generated with Claude Code