tests(react-start-basic): wait for results to be available before validating raw stream results #6307
Conversation
📝 WalkthroughWalkthroughTwo files in the raw-stream test suite were updated: the client component now conditionally hides test result blocks by default, and corresponding e2e tests were refactored to explicitly wait for elements to become visible before reading their text content and performing assertions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
|
View your CI Pipeline Execution ↗ for commit 5ccf30d
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
e2e/react-start/basic/tests/raw-stream.spec.ts (1)
29-37: Excellent approach to fixing flaky tests.The new pattern—wait for visibility, read
textContent()once, then perform multiple assertions—effectively addresses the race condition where assertions ran before results were available. The consistent application across all 16 client RPC tests ensures reliability.Optional: Add null check for defensive coding
textContent()can returnnull. While unlikely after a successful visibility wait, adding a null check or assertion would make the tests more robust:const result = await page.getByTestId('test1-result').textContent() +expect(result).not.toBeNull() expect(result).toContain('chunk1chunk2chunk3')Apply this pattern to all affected tests if desired.
Also applies to: 49-58, 70-78, 90-98, 110-118, 128-136, 259-267, 277-284, 294-301, 311-318, 328-335, 474-486, 497-504, 516-528, 614-622, 633-642
e2e/react-start/basic/src/routes/raw-stream/client-call.tsx (1)
12-34: Consider deferring import reordering to a separate PR.The import reordering appears unrelated to fixing the flaky tests described in the PR objectives. While organizational changes can improve code maintainability, including them in a targeted bug-fix PR increases the diff size and makes the functional changes less obvious.
If the reordering serves a specific purpose (e.g., preparing for upcoming changes or resolving a linting issue), consider documenting that rationale in the PR description.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
e2e/react-start/basic/src/routes/raw-stream/client-call.tsxe2e/react-start/basic/tests/raw-stream.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
e2e/react-start/basic/src/routes/raw-stream/client-call.tsxe2e/react-start/basic/tests/raw-stream.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
e2e/react-start/basic/src/routes/raw-stream/client-call.tsxe2e/react-start/basic/tests/raw-stream.spec.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/public/site.webmanifest:2-3
Timestamp: 2025-10-09T12:59:14.842Z
Learning: In e2e test fixtures (files under e2e directories), empty or placeholder values in configuration files like site.webmanifest are acceptable and should not be flagged unless the test specifically validates those fields.
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
e2e/react-start/basic/src/routes/raw-stream/client-call.tsx
📚 Learning: 2025-12-25T13:04:55.492Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 6215
File: e2e/react-start/custom-basepath/package.json:13-17
Timestamp: 2025-12-25T13:04:55.492Z
Learning: In the TanStack Router repository, e2e test scripts are specifically designed to run in CI (which uses a Unix environment), so Unix-specific commands (like `rm -rf`, `&` for backgrounding, and direct environment variable assignments without `cross-env`) are acceptable in e2e test npm scripts.
Applied to files:
e2e/react-start/basic/src/routes/raw-stream/client-call.tsx
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
e2e/react-start/basic/src/routes/raw-stream/client-call.tsx
📚 Learning: 2025-10-09T12:59:02.129Z
Learnt from: hokkyss
Repo: TanStack/router PR: 5418
File: e2e/react-start/custom-identifier-prefix/src/styles/app.css:19-21
Timestamp: 2025-10-09T12:59:02.129Z
Learning: In e2e test directories (paths containing `e2e/`), accessibility concerns like outline suppression patterns are less critical since the code is for testing purposes, not production use.
Applied to files:
e2e/react-start/basic/tests/raw-stream.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (1)
e2e/react-start/basic/src/routes/raw-stream/client-call.tsx (1)
91-96: Conditional visibility enables reliable test synchronization.Adding
className={results.testX ? '' : 'hidden'}to all test result blocks is the UI-side solution that makes the test fix work. By hiding elements until results are available, the tests can now reliably wait for visibility as a signal that data is ready to validate.Also applies to: 116-121, 141-146, 166-171, 195-200, 220-225, 257-262, 285-290, 313-318, 343-348, 371-376, 410-415, 438-443, 474-479, 508-513, 545-550
The raw stream tests in react-start-basic has some flaky behavior in that it tries to validate the results at times prior to it being available.
This PR attempts to address it by only displaying the results when available and in turn waiting for the specific element to be displayed. This should provide us a bit more accuracy in validating only after the results are available.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.