testing: cover feedback widget behavior#383
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 5 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTest file refactored to shift assertion methodology from structural VNode property checks to integration-style assertions validating rendered text output. Three widget test suites updated: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/widgets/__tests__/infoWidgets.test.ts (2)
21-32: Loosen exact-output assertions to avoid brittle failures.Line 31 and Lines 63-68 currently assert exact full output shape. That can fail on harmless rendering/layout text changes unrelated to the behavior under test. Prefer presence/absence checks for contract-relevant strings only.
Suggested refactor
- assert.equal(output.trim(), "[Retry]"); + assert.equal(output.includes("[Retry]"), true);- const visibleLines = output - .split("\n") - .map((line) => line.trim()) - .filter((line) => line.length > 0); - - assert.deepEqual(visibleLines, ["✗ Oops", "boom"]); + assert.equal(output.includes("✗ Oops"), true); + assert.equal(output.includes("boom"), true); + assert.equal(output.includes("line1"), false); + assert.equal(output.includes("line2"), false);Also applies to: 63-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/widgets/__tests__/infoWidgets.test.ts` around lines 21 - 32, The test currently uses an exact equality assertion on the full rendered output (using createTestRenderer().render(ui.empty(...)).toText() and assert.equal(output.trim(), "[Retry]") ), which is brittle; instead change the assertion to check only the contract-relevant substring(s) (e.g. assert that output includes "[Retry]" or that it contains the action label and/or absence of title/description) so layout/whitespace changes won't break the test; update this test and the similar assertions around the other case (the assertions at lines 63-68) to use inclusion/regex/containment checks (e.g. assert.include/assert.match/assert.ok(output.includes(...))) against output.trim() rather than exact equality.
7-7: Add at least one multi-viewport pass for responsive confidence.All cases render only at
40x10. Consider running these assertions across a second viewport (for example80x24) to catch wrapping/truncation regressions.As per coding guidelines,
**/*.test.{ts,tsx}should “Test responsive behavior by validating UI at multiple viewport sizes.”Also applies to: 22-22, 35-35, 54-54, 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/widgets/__tests__/infoWidgets.test.ts` at line 7, Current tests only render via createTestRenderer({ viewport: { cols: 40, rows: 10 } }), so add a second viewport run (e.g., { cols: 80, rows: 24 }) and assert the same expectations to catch wrapping/truncation; refactor the test(s) to iterate over an array of viewports (using createTestRenderer for each) and run the existing assertions/snapshots inside that loop for the test cases in infoWidgets.test.ts (referencing the existing createTestRenderer usage and the specific test blocks around lines where assertions are made) so each case validates both small and larger viewports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/widgets/__tests__/infoWidgets.test.ts`:
- Around line 21-32: The test currently uses an exact equality assertion on the
full rendered output (using createTestRenderer().render(ui.empty(...)).toText()
and assert.equal(output.trim(), "[Retry]") ), which is brittle; instead change
the assertion to check only the contract-relevant substring(s) (e.g. assert that
output includes "[Retry]" or that it contains the action label and/or absence of
title/description) so layout/whitespace changes won't break the test; update
this test and the similar assertions around the other case (the assertions at
lines 63-68) to use inclusion/regex/containment checks (e.g.
assert.include/assert.match/assert.ok(output.includes(...))) against
output.trim() rather than exact equality.
- Line 7: Current tests only render via createTestRenderer({ viewport: { cols:
40, rows: 10 } }), so add a second viewport run (e.g., { cols: 80, rows: 24 })
and assert the same expectations to catch wrapping/truncation; refactor the
test(s) to iterate over an array of viewports (using createTestRenderer for
each) and run the existing assertions/snapshots inside that loop for the test
cases in infoWidgets.test.ts (referencing the existing createTestRenderer usage
and the specific test blocks around lines where assertions are made) so each
case validates both small and larger viewports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5ddadf5-6eb3-4b0c-8ba2-54280c3cbe34
📒 Files selected for processing (1)
packages/core/src/widgets/__tests__/infoWidgets.test.ts
RtlZeroMemory
force-pushed
the
feature/testing-feedback-widget-behavior
branch
from
4584ce5 to
7ef8fba
Compare
1 participant
Summary
infoWidgetstests around visible behavior for public feedback widgetsempty,errorDisplay, andcalloutat widget-render fidelity instead of prop-shape assertionsFamilies and systems covered
empty,errorDisplay, andcalloutTests added, rewritten, removed
packages/core/src/widgets/__tests__/infoWidgets.test.tsfrom prop-shape checks to visible-output behavior assertionsImplementation bugs fixed
Validation
./node_modules/.bin/tsc -b packages/core/tsconfig.json --pretty falsenode --test packages/core/dist/widgets/__tests__/infoWidgets.test.js packages/core/dist/widgets/__tests__/basicWidgets.render.test.js packages/core/dist/app/__tests__/resilience.test.jsExplicit gaps left out
calloutdefault icon and tone mapping remain intentionally unclaimederrorBoundaryrecovery behavior stays covered by the existing app-level resilience suite rather than this widget-render sliceSummary by CodeRabbit