Repository navigation
[Remove] brittle renderer source-text tests - #434
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe test suite removes brittle Assessment against linked issues
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This test-only cleanup removes brittle renderer source assertions while preserving direct behavior tests; no production or user-visible behavior changes are indicated. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
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 review |
✅ Action performedReview finished.
|
73eb662 to
c9c4d6e
Compare
c9c4d6e to
c508697
Compare
Why
These tests inspect raw
src/renderer/index.jsxtext, so harmless JSX restructuring can fail the suite without changing user-facing behavior. Issue #426 keeps the directly testable renderer decisions covered through their pure modules instead.What changes
Remove source-text assertions from the site URL, changes note, and PR-state unit suites, along with imports used only by those assertions. Direct behavior tests for
site-urls.cjs,changes-note.cjs, andpr-state.cjsremain; no production code changes.How to test this
Platforms: any — this changes unit-test coverage only and has no user-visible surface.
Starting state:
node --test tests/unit/site-urls.test.cjs tests/unit/changes-note.test.cjs tests/unit/pr-state.test.cjs.npm run lintandnpm test.Expected result:
The focused suite reports 34 passing tests, lint succeeds, and the full suite succeeds.
What must not have happened:
The pure-function behavior checks for URL construction, discard-note state, and PR badge state must not be removed. No application code or other test suites should change.
Risks and limitations
No self-review findings. This intentionally stops checking
index.jsxcall-site structure and link/control placement by literal source text; the retained tests validate the pure renderer decisions those source-text checks duplicated. No manual UI test applies because there is no user-visible change.Related
Fixes #426
Design decisions and alternatives considered
Removed only the tests that read
index.jsxas text (including literal counts, ordering, and regex checks). Kept tests that call pure helper functions directly, because they exercise behavior rather than implementation spelling or layout.Review outcome (required — see AGENTS.md)
0 [fix here] · 0 [follow-up]. Reviewed the deletion-only diff across architecture, security, performance, cross-platform behavior, and test coverage after
npm run lintandnpm test; no findings.Implementation notes
Removed 125 lines across
tests/unit/site-urls.test.cjs,tests/unit/changes-note.test.cjs, andtests/unit/pr-state.test.cjs. The branch commit is73eb662.