refactor: implement waitFor pattern to eliminate flaky E2E tests#61
Conversation
Implement waitFor utility based on ink-testing-library PR #27 to eliminate flaky E2E tests. This addresses issue #59 by replacing fixed delay() calls with condition-based polling. Changes: - Add waitFor function to test-utils.ts with configurable timeout and interval - Add waitForContent/waitForNotContent helpers to test-interaction-helpers.ts - Replace all delay() calls in test files with appropriate waitFor patterns - Improve test reliability for large file sets (100 files) with extended timeouts - Add proper error handling for missing directories during test cleanup - Adjust timing expectations in tests to handle CI environment variability The waitFor implementation polls a condition function at regular intervals until it passes or times out, providing more reliable async testing than fixed delays. @see vadimdemedes/ink-testing-library#27
Remove flaky timing-dependent tests from waitFor implementation and reorganize E2E tests to focus on actually testable user behaviors. Changes: - Remove timing-dependent waitFor tests that check internal implementation - Keep only essential waitFor behavior tests (timeout, error handling) - Remove 5 skipped E2E tests that cannot work due to test environment limitations - Convert ESC key test to focus on search clearing (testable behavior) - Clean up debug comments and workarounds from E2E tests - Simplify main E2E test to focus on launch, search, and navigation The menu interaction tests are skipped due to ink-testing-library not supporting focus management. These features are tested at component level instead (MenuActions.test.tsx).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @nyatinte, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly improves the reliability of the test suite, particularly for End-to-End (E2E) tests, by introducing a robust waitFor utility pattern. This new approach replaces fixed delay() calls with condition-based polling, ensuring tests wait for the application state to genuinely reflect changes rather than relying on arbitrary timings. This change makes tests more robust against timing issues and allows them to adapt to varying execution speeds, leading to a more stable and trustworthy test environment.
Highlights
- Introduced
waitForUtility: A newwaitForfunction has been implemented insrc/test-utils.tsthat polls a given condition until it's met or a timeout occurs, making asynchronous test assertions more reliable and eliminating flakiness. - Replaced Fixed Delays: All instances of
delay()calls across various test files have been replaced with the newwaitForpattern, ensuring tests wait for the application state to genuinely reflect changes rather than relying on arbitrary, fixed timings. - Enhanced Test Interaction Helpers: New
waitForContentandwaitForNotContenthelper functions were added tosrc/test-interaction-helpers.tsto provide a more semantic and robust way to wait for specific UI content to appear or disappear in tests. - Test Suite Refinement: The E2E test suite has been cleaned up by removing tests that relied on internal implementation details or were untestable due to
ink-testing-librarylimitations. An ESC key test was also refocused on testable behavior, leading to a more focused and reliable test suite.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
commit: |
WalkthroughThis update systematically replaces fixed time-based delays in test suites with a robust Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant UI
participant waitFor
loop For each async UI check
Test->>waitFor: waitFor(() => UI state condition)
waitFor->>UI: Query current output/state
alt Condition met
waitFor-->>Test: Resolve
else Not met
waitFor->>waitFor: Retry until timeout
alt Timeout
waitFor-->>Test: Throw error
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to replace fixed delays with a waitFor pattern, improving test reliability. However, there are a few instances where the waitFor utility is used incorrectly, potentially re-introducing flakiness. Additionally, some dead code was identified and should be removed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/e2e.test.tsx (1)
55-57: Verify content verification calls are consistent.The test uses both
interaction.verifyContent('Claude Files')immediately after waiting for the same content. This might be redundant sincewaitForContentalready confirms the content exists.Consider removing the redundant verification:
- // Verify initial state - interaction.verifyContent('Claude Files'); + // Content already verified by waitForContent above
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/boundary.test.tsx(8 hunks)src/components/FileList/MenuActions.test.tsx(2 hunks)src/components/FileList/MenuActions/ThemedConfirmInput.test.tsx(9 hunks)src/e2e.test.tsx(6 hunks)src/hooks/useFileNavigation.test.tsx(12 hunks)src/test-interaction-helpers.ts(3 hunks)src/test-keyboard-helpers.ts(1 hunks)src/test-utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: All TypeScript code must use 'type' instead of 'interface' for type definitions.
Regular (non-component) functions must be defined using arrow function syntax.
Avoid default exports except for page components.
Never use the 'any' type; it is strictly forbidden.
Avoid 'as' type assertions; use proper type guards instead.
Remove unused imports and exports immediately to keep dependencies clean.
All TypeScript code must pass with zero type errors (strict mode enforced).
All code must pass Biome lint/format checks with zero errors.
All code must have zero unused dependencies, exports, or types (enforced by Knip).
All code must use immutable design with 'readonly' properties throughout TypeScript code.
All optional properties in TypeScript must use 'exactOptionalPropertyTypes: true' (no '| undefined' on optional props).
Array access in TypeScript must account for 'noUncheckedIndexedAccess: true' (array access returns 'T | undefined').
All code paths in TypeScript functions must return a value ('noImplicitReturns: true').
Files:
src/test-keyboard-helpers.tssrc/components/FileList/MenuActions/ThemedConfirmInput.test.tsxsrc/test-interaction-helpers.tssrc/components/FileList/MenuActions.test.tsxsrc/hooks/useFileNavigation.test.tsxsrc/e2e.test.tsxsrc/boundary.test.tsxsrc/test-utils.ts
src/components/**/*.test.tsx
📄 CodeRabbit Inference Engine (CLAUDE.md)
All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Files:
src/components/FileList/MenuActions/ThemedConfirmInput.test.tsxsrc/components/FileList/MenuActions.test.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (CLAUDE.md)
React components must be defined using the 'function' declaration syntax.
Files:
src/components/FileList/MenuActions/ThemedConfirmInput.test.tsxsrc/components/FileList/MenuActions.test.tsxsrc/hooks/useFileNavigation.test.tsxsrc/e2e.test.tsxsrc/boundary.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
All tests must pass with 100% success rate for React components and business logic.
Files:
src/components/FileList/MenuActions/ThemedConfirmInput.test.tsxsrc/components/FileList/MenuActions.test.tsxsrc/hooks/useFileNavigation.test.tsxsrc/e2e.test.tsxsrc/boundary.test.tsx
src/hooks/*.test.tsx
📄 CodeRabbit Inference Engine (CLAUDE.md)
All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Files:
src/hooks/useFileNavigation.test.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
src/test-keyboard-helpers.ts (2)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
src/components/FileList/MenuActions/ThemedConfirmInput.test.tsx (4)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.test.{ts,tsx} : All tests must pass with 100% success rate for React components and business logic.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
src/test-interaction-helpers.ts (3)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
src/components/FileList/MenuActions.test.tsx (7)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.test.{ts,tsx} : All tests must pass with 100% success rate for React components and business logic.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun supports the ECMAScript resource management proposal including the await using syntax, so examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun has supported the ECMAScript resource management proposal including the using and await using syntax since January 2024 (merged in PR #8151). Examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly in Bun.
src/hooks/useFileNavigation.test.tsx (9)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun supports the ECMAScript resource management proposal including the await using syntax, so examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.test.{ts,tsx} : All tests must pass with 100% success rate for React components and business logic.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun has supported the ECMAScript resource management proposal including the using and await using syntax since January 2024 (merged in PR #8151). Examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly in Bun.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Avoid 'as' type assertions; use proper type guards instead.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : All code must have zero unused dependencies, exports, or types (enforced by Knip).
src/e2e.test.tsx (9)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.test.{ts,tsx} : All tests must pass with 100% success rate for React components and business logic.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : All code must have zero unused dependencies, exports, or types (enforced by Knip).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Avoid default exports except for page components.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun supports the ECMAScript resource management proposal including the await using syntax, so examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: The build must complete without errors, producing a clean output in the 'dist/' directory.
src/boundary.test.tsx (8)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/components/**/*.test.tsx : All React components must be tested, with tests co-located in files ending with '.test.tsx' alongside the component files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/hooks/*.test.tsx : All React hooks must be tested, with tests co-located in files ending with '.test.tsx' alongside the hook files.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.test.{ts,tsx} : All tests must pass with 100% success rate for React components and business logic.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : All code must pass Biome lint/format checks with zero errors.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Avoid 'as' type assertions; use proper type guards instead.
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to **/*.{ts,tsx} : Remove unused imports and exports immediately to keep dependencies clean.
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun supports the ECMAScript resource management proposal including the await using syntax, so examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly.
src/test-utils.ts (2)
Learnt from: CR
PR: nyatinte/ccexp#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T15:51:39.320Z
Learning: Applies to src/_utils.ts : Tests for utility functions must be defined using the InSource Testing pattern (tests live alongside the source code in the same file).
Learnt from: nyatinte
PR: #33
File: CLAUDE.md:265-282
Timestamp: 2025-07-21T18:07:28.502Z
Learning: Bun supports the ECMAScript resource management proposal including the await using syntax, so examples using await using fixture = await createFixture(fileTree); are valid and will compile correctly.
🔇 Additional comments (28)
src/test-utils.ts (2)
29-50: Excellent waitFor implementation with robust error handling.The implementation correctly uses a polling pattern to replace fixed delays, which aligns perfectly with the PR objectives. The error handling properly captures the last error and provides meaningful timeout messages.
66-111: Comprehensive test coverage for waitFor utility.The test suite covers all essential scenarios: immediate success, timeout behavior, error message propagation, and asynchronous state changes. This thorough testing ensures the reliability of the waitFor pattern implementation.
src/test-keyboard-helpers.ts (1)
75-75: Good cleanup removing unused export.This change follows the coding guideline to remove unused exports, keeping the module's API clean while preserving the function's testability through InSource tests.
src/components/FileList/MenuActions.test.tsx (2)
8-8: Proper import replacement for waitFor utility.The change from
delaytowaitForimport aligns with the PR's objective to implement more reliable asynchronous testing patterns.
561-566: Excellent use of waitFor for UI synchronization.This replaces a fixed delay with condition-based waiting, checking for the specific menu selection indicator. The 50ms timeout is appropriate for UI updates, and the error message provides clear debugging information.
src/components/FileList/MenuActions/ThemedConfirmInput.test.tsx (1)
4-4: Import change aligns with waitFor pattern adoption.The import replacement supports the broader refactoring to use waitFor utilities across the test suite.
src/test-interaction-helpers.ts (3)
4-4: Appropriate import addition for waitFor utility.Adding the waitFor import enables the new helper functions while maintaining the existing functionality.
103-141: Excellent helper functions implementing waitFor pattern correctly.Both
waitForContentandwaitForNotContentproperly use the waitFor utility with meaningful condition checking. They provide valuable abstractions for common UI testing scenarios and will improve test reliability.The error messages are clear and helpful for debugging, and the optional timeout/interval parameters provide flexibility.
157-158: Proper integration of new helper functions.The new waitFor-based helpers are correctly added to the returned interface, making them available alongside existing interaction methods.
src/e2e.test.tsx (5)
44-44: Test name accurately reflects simplified scope.The test name change from a generic identifier to "complete flow: launch, search, navigate" clearly indicates the test has been simplified to remove clipboard and menu operations that were problematic in the test environment.
52-52: Excellent replacement of fixed delay with condition-based waiting.The change from
delay()tointeraction.waitForContent('Claude Files')is a significant improvement. This now waits for the actual UI state rather than an arbitrary timeout, making the test more reliable and faster.
70-80: Test assertions improved with proper content verification.The changes from generic output checks to specific content verification using
interaction.verifyContent()andinteraction.assertOutput()provide clearer test intent and better failure messages.
158-161: Clear documentation of test limitations.The comment explaining why menu interaction tests are skipped is helpful for understanding the test coverage gaps and provides guidance on where these features are actually tested (MenuActions.test.tsx).
162-190: New escape key test focuses on testable behavior.The new test for escape key clearing search is well-designed, focusing on behavior that can be reliably tested in the current environment rather than problematic menu interactions.
src/hooks/useFileNavigation.test.tsx (7)
8-8: Proper import of waitFor utility.The import correctly brings in the new
waitForutility that implements the polling pattern to replace fixed delays.
98-103: Excellent waitFor implementation pattern.This waitFor usage perfectly demonstrates the intended pattern: polling until the loading state disappears by throwing an error when the condition isn't met. This is much more reliable than fixed delays.
141-145: Consistent error handling in waitFor conditions.The waitFor pattern is correctly applied here, waiting for the error to be captured rather than using arbitrary delays. The condition properly throws when the expected state hasn't been reached.
190-195: Proper async waiting pattern maintained.The waitFor usage maintains the same reliable pattern throughout the test suite, ensuring consistent behavior across all asynchronous operations.
288-293: Appropriate timeout configuration for complex operations.The addition of a 500ms timeout parameter for the complex project structure test is reasonable given the increased complexity of the scanning operation.
504-507: Minimal delay usage is justified for negative testing.The use of a very short 50ms waitFor delay in this context is appropriate since it's specifically testing that no auto-refresh occurs, making this a legitimate use of a minimal delay.
543-548: Extended timeout for recursive scanning is appropriate.The 2-second timeout for recursive directory scanning is reasonable given the potentially expensive nature of recursive file system operations in test environments.
src/boundary.test.tsx (7)
10-10: Correct import update for new testing utilities.The import correctly replaces
delaywithwaitForandwaitForEffects, aligning with the new testing strategy.
202-202: Appropriate timeout increase for realistic test scenario.Increasing the timeout to 10 seconds for a test handling 99 files is reasonable, as file system operations can be slower in test environments.
204-212: Improved test fixture creates more realistic scenario.The change to create 1 CLAUDE.md file plus 98 test files (total 99) provides a more realistic testing scenario than 100 identical files, and the use of
Object.fromEntriesis a clean approach.
227-239: Robust waitFor implementation with comprehensive conditions.This waitFor implementation is excellent - it checks for multiple possible end states ("Claude Files" or "No Claude files found") and includes appropriate error messages for debugging. The 8-second timeout is reasonable for scanning 99 files.
258-262: Improved error handling in test cleanup.Wrapping
process.chdir(originalCwd)in a try-catch block is a good defensive programming practice, preventing test failures if the directory no longer exists during cleanup.
334-334: Consistent use of waitForContent pattern.The replacement of delay with
interaction.waitForContent('Claude Files')maintains consistency with the testing strategy used throughout the codebase.
383-388: Proper waitFor usage for empty state detection.The waitFor implementation correctly waits for the "No Claude files found" message, handling the empty directory scenario appropriately.
- Replace empty waitFor callbacks with proper condition checking in ThemedConfirmInput tests - Use delay() instead of waitFor for negative testing in useFileNavigation test - Remove unused typeKeys function and its test from test-keyboard-helpers These changes address review feedback on PR #61 to ensure waitFor is used correctly for polling conditions rather than as a simple delay replacement.
- Extract hardcoded timeout value (50ms) to WAIT_FOR_CALLBACK_TIMEOUT constant - Add detailed comments explaining double waitForEffects() calls for React StrictMode - Improve maintainability by centralizing timeout configuration These changes address additional review feedback on PR #61 to enhance test clarity and maintainability.
1 participant
Overview
This PR implements a waitFor utility pattern to replace fixed delay() calls in tests, significantly improving test reliability and eliminating flaky behavior, especially in E2E tests.
Changes
1. Implement waitFor Pattern
2. Clean Up Test Suite
3. Address Skipped Tests
The following E2E tests were skipped due to ink-testing-library not supporting focus management:
flow: copy different path formats- Tests clipboard operations for different path typesflow: escape key behavior in different contexts- Converted to test search clearing onlyflow: error handling when clipboard fails- Edge case for clipboard errorsflow: search during menu display- Edge case behaviorflow: rapid navigation and selection- Stress test scenarioThese menu interaction features are already covered by component-level tests in
MenuActions.test.tsx, ensuring the functionality remains tested despite the E2E limitations.Benefits
Technical Details
The waitFor implementation polls a condition function at regular intervals until it passes or times out:
Related
Test Results
All 392 tests pass with improved reliability across different environments.
Summary by CodeRabbit
Tests
Chores