test: dedupe shared test setup (console spies, program, channel fixture)#248
Merged
Conversation
Replace ~209 inline `vi.spyOn(console, ...).mockImplementation(() => {})`
spies across 25 test files with a shared `captureConsole()` helper that
silences output and auto-restores via onTestFinished, removing the
redundant manual mockRestore() calls.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace 24 identical local `createProgram()` definitions with a shared `createTestProgram(register)` helper; each test keeps a one-line wrapper so call sites are unchanged. Drops the now-unused commander imports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace four byte-identical `sampleChannel` literals (add/remove/set/ members) with a shared `createChannelFixture()` factory in __fixtures__, matching the existing accounts fixture pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
reviewed
May 22, 2026
Member
doistbot
left a comment
There was a problem hiding this comment.
This PR effectively cleans up the test suite by deduplicating console spies, program initialization, and channel fixtures into shared helpers. This provides a great boost to maintainability by dropping nearly 400 lines of boilerplate while thoughtfully avoiding premature abstractions. A few minor adjustments are needed to ensure the new test helpers directory is properly excluded from the published build output and to strictly type the channel fixture overrides so we maintain the safety of the original inline literals.
Add src/test-helpers to tsconfig.build.json excludes so the published dist/ no longer pulls in vitest-importing helpers. Type createChannelFixture against the SDK Channel shape (Partial<Channel> overrides, Channel return) to keep the type safety of the inline literals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gnapse
approved these changes
May 22, 2026
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
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 + dedupe of repeated test scaffolding across the suite. Three focused commits, each its own concern:
captureConsole()(src/test-helpers/console.ts) — replaces ~209 inlinevi.spyOn(console, ...).mockImplementation(() => {})spies across 25 files and removes ~159 manualmockRestore()calls (auto-restores viaonTestFinished).createTestProgram(register)(src/test-helpers/program.ts) — collapses 24 identical localcreateProgram()definitions; call sites unchanged via a one-line wrapper. Drops now-unused commander imports.createChannelFixture()(src/lib/__fixtures__/channels.ts) — replaces 4 byte-identicalsampleChannelliterals (channel add/remove/set/members), matching the existing accounts-fixture pattern.Net: −367 lines (29 files), no behavior change.
Deliberately not done
makeTwistClientMock— the per-command client mocks are each a different SDK subset; a generic helper would be a no-op{...overrides}wrapper (premature abstraction).apiMocks/refsMocksfactories — real dup exists in the 4 channel-membership files, butvi.hoistedruns before imports so a shared factory can't be imported into it (TDZ); the__mocks__/alternative would force all 16 api-mocking files onto one global mock surface.Test plan
npm run type-checkcleannpm run lint:checkcleannpm test— 677 passed (43 files)🤖 Generated with Claude Code