fix(test): replace fixed-delay waits with polling in MainConfig walkthrough tests#126
Conversation
…hrough tests The walkthrough tests waited a fixed 100ms after each stdin write, assuming Ink's re-render always completed in that window. Under CI load (observed on Node 24 ubuntu-latest) this could flake before a re-render caught up. Replace the sleep with vi.waitFor polling for the expected frame content, and add an explicit wait for the masked password field before submitting the API key so keystrokes are guaranteed to have reached component state.
There was a problem hiding this comment.
Code Review
This pull request refactors the CLI integration tests in ink/main.test.tsx to eliminate flakiness under CI load. It replaces fixed-duration timeouts with asynchronous polling helpers (waitForFrame and waitForMaskedInput) that wait for specific frames to render before sending keystrokes. The review feedback correctly identifies a potential robustness issue where lastFrame() might return undefined during initial renders, which would cause a TypeError in the assertions. The suggested improvement to use nullish coalescing (?? '') is highly recommended to ensure clean assertion failures.
| const waitForFrame = (lastFrame: () => string | undefined, text: string) => | ||
| vi.waitFor(() => expect(lastFrame()).toContain(text)); |
There was a problem hiding this comment.
If lastFrame() returns undefined (which can happen during initial render or before the first frame is drawn), calling expect(undefined).toContain(text) can throw a TypeError in some test runners because undefined is not iterable/a string. Using the nullish coalescing operator ?? '' ensures that we always perform a string assertion, which provides a much cleaner assertion failure message if the test times out, rather than a generic TypeError.
| const waitForFrame = (lastFrame: () => string | undefined, text: string) => | |
| vi.waitFor(() => expect(lastFrame()).toContain(text)); | |
| const waitForFrame = (lastFrame: () => string | undefined, text: string) => | |
| vi.waitFor(() => expect(lastFrame() ?? '').toContain(text)); |
| const waitForMaskedInput = (lastFrame: () => string | undefined, text: string) => | ||
| vi.waitFor(() => expect(lastFrame()).toContain('*'.repeat(text.length))); |
There was a problem hiding this comment.
Similarly to waitForFrame, if lastFrame() returns undefined, calling expect(undefined).toContain(...) can throw a TypeError. Using ?? '' ensures a safe string assertion and cleaner error messages upon timeout.
| const waitForMaskedInput = (lastFrame: () => string | undefined, text: string) => | |
| vi.waitFor(() => expect(lastFrame()).toContain('*'.repeat(text.length))); | |
| const waitForMaskedInput = (lastFrame: () => string | undefined, text: string) => | |
| vi.waitFor(() => expect(lastFrame() ?? '').toContain('*'.repeat(text.length))); |
|
🎉 This PR is included in version 0.16.24 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Fixes the flaky
ink/main.test.tsx > MainConfig > completes the walkthrough for OpenAI and updates .envtest that failed CI on #125 (Node 24, ubuntu-latest).OpenAI,Anthropic,Google,Ollama) waited a fixed100msafter every stdin write, assuming Ink's re-render always completes in that window. Under CI load this isn't guaranteed, so the next assertion could read a stale frame.vi.waitForpolling for the expected frame content — deterministic and no slower than necessary.*per character), so waiting for the next step's text after typing the key doesn't confirm the keystrokes actually landed in component state before Enter is sent. Added an explicit wait for the masked value's length before submitting.Test plan
npx vitest run ink/main.test.tsx— all 6 tests passnpx vitest run) — 345/345 passnpm run lintandnpm run format— clean🤖 Generated with Claude Code