|
| 1 | +# Test Isolation Fixes |
| 2 | + |
| 3 | +Two bugs. Two files touched. |
| 4 | + |
| 5 | +--- |
| 6 | + |
| 7 | +## 1. `packages/modifiers-napi/src/__tests__/index.test.ts` |
| 8 | + |
| 9 | +**Problem:** `mock.module('bun:ffi', ...)` is declared at module scope with no cleanup. |
| 10 | +Bun reuses workers across files in a full suite run, so the mock bleeds into other |
| 11 | +files that import anything touching `bun:ffi`. When `index.test.ts` runs _after_ |
| 12 | +another file has already loaded `bun:ffi` into the module registry, the module-level |
| 13 | +mock declaration loses the race and the test gets the wrong module — or nothing at all. |
| 14 | + |
| 15 | +**Fix:** Add `afterAll(() => mock.restore())`. |
| 16 | + |
| 17 | +```typescript |
| 18 | +// packages/modifiers-napi/src/__tests__/index.test.ts |
| 19 | + |
| 20 | +import { afterAll, beforeEach, describe, expect, mock, test } from 'bun:test' |
| 21 | + |
| 22 | +let ffiShouldThrow = false |
| 23 | +let nativeFlags = 0 |
| 24 | +let dlopenCalls = 0 |
| 25 | + |
| 26 | +mock.module('bun:ffi', () => ({ |
| 27 | + FFIType: { |
| 28 | + i32: 0, |
| 29 | + u64: 0, |
| 30 | + }, |
| 31 | + dlopen: () => { |
| 32 | + dlopenCalls++ |
| 33 | + if (ffiShouldThrow) { |
| 34 | + throw new Error('ffi load failed') |
| 35 | + } |
| 36 | + return { |
| 37 | + symbols: { |
| 38 | + CGEventSourceFlagsState: () => nativeFlags, |
| 39 | + }, |
| 40 | + } |
| 41 | + }, |
| 42 | +})) |
| 43 | + |
| 44 | +// ✅ NEW: restore the bun:ffi mock after this file finishes |
| 45 | +// so it doesn't bleed into other files running in the same worker |
| 46 | +afterAll(() => { |
| 47 | + mock.restore() |
| 48 | +}) |
| 49 | + |
| 50 | +beforeEach(() => { |
| 51 | + ffiShouldThrow = false |
| 52 | + nativeFlags = 0 |
| 53 | + dlopenCalls = 0 |
| 54 | +}) |
| 55 | + |
| 56 | +// ... rest of file unchanged |
| 57 | +``` |
| 58 | + |
| 59 | +--- |
| 60 | + |
| 61 | +## 2. `src/utils/staticRender.tsx` |
| 62 | + |
| 63 | +**Problem:** `await instance.waitUntilExit()` hangs indefinitely when Ink's reconciler |
| 64 | +commit phase doesn't complete. `waitUntilExit()` only resolves when `exit()` is called |
| 65 | +inside `useLayoutEffect` — but if `@anthropic/ink`'s `wrappedRender` has stale |
| 66 | +process-level state from a previous test file (singleton reconciler, leaked |
| 67 | +`process.on('exit')` handlers, a cached fiber root), React never commits, `useLayoutEffect` |
| 68 | +never fires, and the whole render silently hangs until the 10s test timeout kills it. |
| 69 | + |
| 70 | +This only surfaces in the full suite because Bun shares workers across files sequentially. |
| 71 | +Running the file alone gives it a clean worker with no prior Ink state. |
| 72 | + |
| 73 | +**Fix:** Race `waitUntilExit()` against a 3s safety timeout. On timeout, call |
| 74 | +`instance.unmount()` to force Ink cleanup and throw a descriptive error instead of |
| 75 | +a silent 10s hang. |
| 76 | + |
| 77 | +Also: explicitly remove the stream `data` listener and destroy the stream after render. |
| 78 | +Without this, the PassThrough stream stays open and its listener accumulates garbage |
| 79 | +across calls in the same process lifetime. |
| 80 | + |
| 81 | +```typescript |
| 82 | +// src/utils/staticRender.tsx |
| 83 | + |
| 84 | +import * as React from 'react'; |
| 85 | +import { useLayoutEffect } from 'react'; |
| 86 | +import { PassThrough } from 'stream'; |
| 87 | +import stripAnsi from 'strip-ansi'; |
| 88 | +import { wrappedRender as render, useApp } from '@anthropic/ink'; |
| 89 | + |
| 90 | +function RenderOnceAndExit({ children }: { children: React.ReactNode }): React.ReactNode { |
| 91 | + const { exit } = useApp(); |
| 92 | + |
| 93 | + useLayoutEffect(() => { |
| 94 | + const timer = setTimeout(exit, 0); |
| 95 | + return () => clearTimeout(timer); |
| 96 | + }, [exit]); |
| 97 | + |
| 98 | + return <>{children}</>; |
| 99 | +} |
| 100 | + |
| 101 | +const SYNC_START = '\x1B[?2026h'; |
| 102 | +const SYNC_END = '\x1B[?2026l'; |
| 103 | + |
| 104 | +function extractFirstFrame(output: string): string { |
| 105 | + const startIndex = output.indexOf(SYNC_START); |
| 106 | + if (startIndex === -1) return output; |
| 107 | + |
| 108 | + const contentStart = startIndex + SYNC_START.length; |
| 109 | + const endIndex = output.indexOf(SYNC_END, contentStart); |
| 110 | + if (endIndex === -1) return output; |
| 111 | + |
| 112 | + return output.slice(contentStart, endIndex); |
| 113 | +} |
| 114 | + |
| 115 | +export async function renderToAnsiString(node: React.ReactNode, columns?: number): Promise<string> { |
| 116 | + let output = ''; |
| 117 | + |
| 118 | + const stream = new PassThrough(); |
| 119 | + if (columns !== undefined) { |
| 120 | + (stream as unknown as { columns: number }).columns = columns; |
| 121 | + } |
| 122 | + |
| 123 | + // ✅ NEW: named handler so we can remove it after render |
| 124 | + const dataHandler = (chunk: Buffer) => { |
| 125 | + output += chunk.toString(); |
| 126 | + }; |
| 127 | + stream.on('data', dataHandler); |
| 128 | + |
| 129 | + const instance = await render(<RenderOnceAndExit>{node}</RenderOnceAndExit>, { |
| 130 | + stdout: stream as unknown as NodeJS.WriteStream, |
| 131 | + patchConsole: false, |
| 132 | + }); |
| 133 | + |
| 134 | + // ✅ NEW: race waitUntilExit against a 3s hard limit. |
| 135 | + // If Ink's reconciler is stuck (stale worker state from a prior test file), |
| 136 | + // this surfaces a real error instead of silently hanging for 10s. |
| 137 | + await Promise.race([ |
| 138 | + instance.waitUntilExit(), |
| 139 | + new Promise<void>((_, reject) => |
| 140 | + setTimeout(() => { |
| 141 | + instance.unmount(); |
| 142 | + reject( |
| 143 | + new Error( |
| 144 | + '[staticRender] Ink render did not exit within 3s — wrappedRender may have stale process state from a prior test file', |
| 145 | + ), |
| 146 | + ); |
| 147 | + }, 3000), |
| 148 | + ), |
| 149 | + ]); |
| 150 | + |
| 151 | + // ✅ NEW: clean up the stream so it doesn't accumulate across calls |
| 152 | + stream.off('data', dataHandler); |
| 153 | + stream.destroy(); |
| 154 | + |
| 155 | + return extractFirstFrame(output); |
| 156 | +} |
| 157 | + |
| 158 | +export async function renderToString(node: React.ReactNode, columns?: number): Promise<string> { |
| 159 | + const output = await renderToAnsiString(node, columns); |
| 160 | + return stripAnsi(output); |
| 161 | +} |
| 162 | +``` |
| 163 | + |
| 164 | +--- |
| 165 | + |
| 166 | +## If AutofixProgress tests still hang after this |
| 167 | + |
| 168 | +The 3s safety net will stop the silent timeout and give you a real error message. |
| 169 | +If you're still seeing the hang, the root is inside `wrappedRender` from `@anthropic/ink`. |
| 170 | +Look for any of these in that file: |
| 171 | + |
| 172 | +- A module-level singleton (cached app instance, fiber root, reconciler) |
| 173 | +- `process.on('exit')` / `process.on('SIGTERM')` handlers added on each `render()` call but never removed |
| 174 | +- A global `stdout` patch applied once and assumed fresh on every call |
| 175 | + |
| 176 | +Share that file and the exact line can be pinpointed. |
| 177 | + |
| 178 | +--- |
| 179 | + |
| 180 | +## Summary |
| 181 | + |
| 182 | +| File | Change | Why | |
| 183 | +| ------------------ | ------------------------------------------------------- | ------------------------------------------------ | |
| 184 | +| `index.test.ts` | Add `afterAll(() => mock.restore())` | Stops `bun:ffi` mock bleeding into other workers | |
| 185 | +| `staticRender.tsx` | Race `waitUntilExit()` with 3s timeout + stream cleanup | Surfaces real errors instead of silent 10s hangs | |
0 commit comments