Skip to content

fix(test): stop use-input test from leaking a global stdin mock - #1501

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
kevincodex1:fix/use-input-test-mock-leak
Jun 3, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
kevincodex1:fix/use-input-test-mock-leak

Conversation

@kevincodex1

@kevincodex1 kevincodex1 commented Jun 3, 2026 •

Copy link
Copy Markdown
Member

The use-input.test.ts added in #1198 broke the full bun test run two ways:

  1. It imported @testing-library/react-hooks, which was never installed and is React 16/17/18-only (incompatible with this repo's React 19), so the file errored on load.
  2. Its top-level vi.mock('./use-stdin.js', …) registered a module mock that leaks across every later file in the same bun test process. The fake eventEmitter's .on was a no-op, so useInput silently registered no listener and dropped all keystrokes — surfacing as timeouts in MonitorPermissionRequest and the agent-menu/wizard TextInput tests (which passed in isolation but failed in the full suite).

Rewrite the test to inject the stdin handle via StdinContext.Provider (no leaking global mock) and render through the real ink root (no @testing-library/react-hooks). Drop the now-dead react-hooks and react-test-renderer devDependencies and reconcile the lockfile.

Full suite: 3429 pass, 0 fail (was 9 fail + 1 error).

Summary by CodeRabbit

  • Chores

    • Updated development dependencies.
  • Tests

    • Refactored test infrastructure for improved maintainability.

The use-input.test.ts added in Twigpine#1198 broke the full `bun test` run two ways:

1. It imported `@testing-library/react-hooks`, which was never installed and
   is React 16/17/18-only (incompatible with this repo's React 19), so the
   file errored on load.
2. Its top-level `vi.mock('./use-stdin.js', …)` registered a module mock that
   leaks across every later file in the same `bun test` process. The fake
   eventEmitter's `.on` was a no-op, so `useInput` silently registered no
   listener and dropped all keystrokes — surfacing as timeouts in
   MonitorPermissionRequest and the agent-menu/wizard TextInput tests (which
   passed in isolation but failed in the full suite).

Rewrite the test to inject the stdin handle via StdinContext.Provider (no
leaking global mock) and render through the real ink root (no
@testing-library/react-hooks). Drop the now-dead react-hooks and
react-test-renderer devDependencies and reconcile the lockfile.

Full suite: 3429 pass, 0 fail (was 9 fail + 1 error).

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
@coderabbitai

coderabbitai Bot commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6057be2c-2083-48ea-b01e-65ad4368a9a6

📥 Commits

Reviewing files that changed from the base of the PR and between 3bf6ccd and 0371e06.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • package.json
  • src/ink/hooks/use-input.test.ts
💤 Files with no reviewable changes (1)
  • package.json

📝 Walkthrough

Walkthrough

This PR removes the @testing-library/react-hooks and react-test-renderer dev dependencies and rewrites the useInput hook test suite with a custom test harness that mounts the hook in a real React root with StdinContext.Provider injection, replacing fake-timer assertions with real async timing.

Changes

Test Harness Migration

Layer / File(s) Summary
Dependency cleanup
package.json
Remove @testing-library/react-hooks and react-test-renderer from devDependencies.
Test harness and imports
src/ink/hooks/use-input.test.ts
New test imports replace testing-library mocks; add Bun root/PassThrough stream creation; define fake StdinContext value with setRawMode spy and real EventEmitter; implement renderProbe helper that mounts Probe component and exposes rerender/unmount; switch from fake timers to async tick() using Bun.sleep(5); rework suite setup/teardown to use bun:test mock spy lifecycle.
Test case implementations
src/ink/hooks/use-input.test.ts
Rewrite five test cases to async form: verify setRawMode(true) on mount when active; verify no call when inactive; verify deferred setRawMode(false) after unmount tick; verify cancelled deferred reset on true → false → true transition; verify true → false transition eventually calls deferred reset.

🎯 2 (Simple) | ⏱️ ~10 minutes

A test harness springs to life with a real React root so true,
No fake timers needed, just Bun.sleep and tick() do,
The hook now renders honestly, with context injected just right,
Each test case hops and bounds in async delight,
What once was mocked, now breathes with real-time might! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(test): stop use-input test from leaking a global stdin mock' accurately and specifically summarizes the main issue being fixed—preventing a test mock from leaking across the test suite.
Description check ✅ Passed The description includes the required Summary (what changed and why), Impact (the test suite now passes fully), and Testing sections with verification of the fix working.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@kevincodex1
kevincodex1 merged commit 96ddec7 into Twigpine:main Jun 3, 2026
3 checks passed
hotmanxp pushed a commit to hotmanxp/openclaude that referenced this pull request Jun 5, 2026
Upstream 343cd1a..1204fe2, applied 6 of 8 KEEP candidates in tier 1:

  11e46af fix(typecheck): narrow hook event counts (Twigpine#1496)
  2c755d3 fix(typecheck): restore typed add-dir source (Twigpine#1504)
  2bed184 perf(attachments): skip skill listings for utility forks (Twigpine#1545)
  47eea3f fix(typecheck): type search UI state (Twigpine#1529)
  96ddec7 fix(test): stop use-input test from leaking a global stdin mock (Twigpine#1501)
  1fc5116 fix(api): tighten reasoning_content heuristic to prevent false-positives (Twigpine#1201)

Notes:
- 2c755d3: react-compiler compiled output (add-dir.tsx) replaced with typed
  source; build re-compiles on next run
- 47eea3f: kept fork-specific // @ts-nocheck at top of GlobalSearchDialog.tsx
  (added by local b40814b, not in upstream); applied type Props + true arg
- 96ddec7: bun.lock regenerated via 'bun install' to reconcile with new
  package.json (devDependencies pruned: react-hooks, react-test-renderer);
  new use-input.test.ts (208 lines) injected via StdinContext.Provider
- 1fc5116: new runtimeMetadata.test.ts (90 lines, regression coverage for
  the segment-boundary heuristic per jatmn review)

Verification:
  baseline:  2223 pass / 23 skip / 4 fail
  after:     2232 pass / 23 skip / 2 fail   (+9 pass, -2 fail)
  typecheck: 90 errors (pre-existing, no new)
  build:     Built v0.16.1

Rebase onto OpenCC v0.14.0 (2292a59).
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 6, 2026
…pine#1501)

Per-file port of upstream 96ddec7. 1 file (use-input.test.ts).

3way conflict at the `tick()` helper (fork had setTimeout-based
5ms wait, upstream now uses `Bun.sleep(5)`): took theirs.
Added `// @ts-nocheck` at line 1 since `@types/bun` is not
installed; runtime is fine, only the type checker complains.

Source: upstream 96ddec7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant