Uh oh!
There was an error while loading. Please reload this page.
docs: record the two example test gaps left by the review fixes - #12
Merged
Conversation
PR #11 shipped two deliberate shortcuts in `examples/order-api` whose reasoning lived only in that PR's description. Both now sit in the "Deferred, deliberately" list, where the rest of this repo's deferrals are tracked, each with the ceiling that stopped it and the trigger to revisit. - The permanent server `'error'` listener has no test of its own. Guarding it needs `vi.mock("node:http")` hoisted above `describe` — `vi.spyOn` cannot patch a node builtin — which breaks Test conventions rule 1 in the one workspace whose spec shape is itself the advice, to cover a mechanism its kernel twin in `probes.spec.ts` already covers. - `closeAfterResponse`'s `headersSent` branch is unreachable through this router, so nothing can exercise it without inventing a streamed route purely to be tested. It exists so the drain's guarantee stays "no reuse" rather than "no reuse where we caught the header in time". Recorded rather than marked inline: this file already states that rationale belongs here and not in comments, and both are deferrals rather than a line needing a guard against a plausible simplification. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s root CLAUDE.md to permanently document two known, deliberate test/coverage gaps in examples/order-api that were previously only captured in PR #11’s description, keeping deferral rationale centralized alongside other documented tradeoffs.
Changes:
- Add a deferral entry explaining why
examples/order-api’s permanent server'error'listener fix lacks a dedicated test (and what would trigger revisiting it). - Add a deferral entry explaining why
closeAfterResponse’sheadersSentbranch remains uncovered (and what would make it coverable).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #11. That PR shipped two deliberate shortcuts in
examples/order-apiwhose reasoning lived only in the PR description — which isexactly where a deferral goes to be forgotten. Both are now in
CLAUDE.md's"Deferred, deliberately" list, alongside the rest of this repo's deferrals.
'error'listenervi.mock("node:http")hoisted abovedescribe, breaking Test conventions rule 1 in the teaching-surface workspace;vi.spyOncannot patch a node builtin (Cannot redefine property: createServer)closeAfterResponse'sheadersSentbranch is uncoveredendWithputswriteHeadandendadjacent, and oRPC serialises these small bodies in one goEach entry records why the gap is acceptable, not just that it exists: the
first is guarded by its kernel twin (
probes.spec.ts→ "does not throw whenthe server emits an error after binding" — the same two lines for the same
reason), and the second costs nothing at the gate because
examples/carries nocoverage threshold.
Recorded here rather than marked inline.
CLAUDE.mdalready states thatrationale belongs in it and not in comments, with one carve-out — a comment
guarding a specific line against a plausible "simplification". The
headersSentbranch already has that comment; neither of these is a line needing a guard, so
both belong in the list.
Docs only: no code, no changeset (
CLAUDE.mdis not in the published tarball —files: ["dist"]).🤖 Generated with Claude Code