test: cover the last of the thirteen, and the three upstream helpers a user can see - #521
Merged
Merged
Conversation
#519 covered eleven of the thirteen components that are ours and had no coverage anywhere. This is the twelfth and last that can be tested on its own — a nineteen-line passthrough over reka-ui's `DialogClose`, used once, by `SidebarLayout`, to close the mobile sidebar. Mounted inside a real `Modal`, because `DialogClose` alone has no dialog to close and the test would be asserting that nothing happens. `open` is bound rather than passed as a literal `true`: a literal is never written back, so the dialog stays open however well the click works — the first draft passed that way and proved nothing. Replacing `DialogClose` with a plain `span` reddens it. Refs #86
A change of direction, decided with the maintainer: we do write tests for code we ship but did not write. A test does not diverge from upstream the way a rewrite does — it lives in `test/`, and `src/` stays byte-identical, so nothing new conflicts on a sync. And upstream having no tests is the argument for writing them rather than against: when we port their changes, ours are the only safety net (#75). The filter is "what a user can see", not coverage. Three helpers qualified, each sitting under something visible and each essentially unexercised: `utils/overlay.ts` — 0% — decides whether a click beside a `Modal`, `Drawer` or `Popover` closes it. Both of its branches exist for a case where the honest answer is wrong: a target that left the DOM between pointerdown and click (a toast dismissing itself on touch), and a click on the overlay's own scrollbar, which is painted outside the client box. Misfiring either way is an overlay that will not close, or one that closes mid-drag. `utils/link.ts` — 44% statements, 0% branches — `isPartiallyEqual` is what `exactQuery: 'partial'` means. The current route carries whatever else is in the URL, and a link declaring `?tab=general` has to stay highlighted through a search term and a page number. Keys the route adds are forgiven; keys the link declares are not. Breaking it shows up as a navigation item highlighting when it should not. `utils/content.ts` — 0% — renames `title` to `label` and `path` to `to` for every entry in a `@nuxt/content` tree, which is to say it builds the sidebar of any docs site built on this kit. Get it wrong and the sidebar is a column of blank, unclickable rows. All three go to 100% of statements and branches; the suite total moves 74.31% to 74.76%. Every branch is mutation-checked — forgiving removed keys as well as added ones, dropping the `isConnected` guard, widening the scrollbar check, removing the `deep` limit, and four more each redden a named test. Not covered, and deliberately: `ai.ts` (streaming-state predicates, visible only inside a chat UI), `virtualizer.ts` (71%, and what is left is measurement arithmetic) and `editor.ts` (12% of 587 lines — a slice of its own, if it is worth one at all). Refs #86
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 free
to 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.
Linked issue
Refs #86.
Type of change
revert(Scope): ...)No
src/changes at all — four new spec files.1.
ModalDialogClose— the last of the thirteen#519 covered eleven of the thirteen components that are ours and had no coverage anywhere. This is the twelfth: a nineteen-line passthrough over reka-ui's
DialogClose, used once, bySidebarLayout, to close the mobile sidebar.Two things the spec had to get right, and one of them it got wrong first:
Modal. On its own,DialogClosehas no dialog to close, and the test would assert that nothing happens.openis bound, not a literaltrue. A literal is never written back, so the dialog stays open however well the click works. The first draft passed exactly that way and proved nothing.Replacing
DialogClosewith a plainspanreddens it.2. A change of direction, and what it cost to establish
The plan recorded on #86 put the nine untested
src/runtime/utilsmodules next. Measured first, that workstream is almost empty — "has no spec named after it" is not the same as uncovered:ai.tsnuxt/ui@v4content.tsoverlay.tseditor.tsas any(see #88)link.tsvirtualizer.tsdashboard.ts,link-keys.ts,prototype-guard.tsprototype-guard.tslooked like the prize — 81 lines, entirely ours, closing two prototype-pollution holes. It is already at 100%, exercised throughget/setandgetAtPath/setAtPath, including the case whereset({}, 'toString.x', 1)reachedObject.prototype.toStringwith no reserved word in the path.So everything left uncovered there is upstream's logic, and the question became whether we test code we ship but did not write. Decided with the maintainer: yes, but only what a user can see. A test does not diverge from upstream the way a rewrite does — it lives in
test/,src/stays byte-identical, and nothing new conflicts on a sync. Upstream having no tests is the argument for writing them: when we port their changes, ours are the only safety net (#75).3. The three that passed that filter
utils/overlay.tsModal,DrawerorPopovercloses itutils/link.tsutils/content.tspointerDownOutsidehas two branches, and both exist for a case where the honest answer — "the pointer went down outside, so close" — is wrong: a target that left the DOM between pointerdown and click (a toast dismissing itself on touch), and a click on the overlay's own scrollbar, which is painted outside the client box. Misfiring either way gives an overlay that will not close, or one that closes mid-drag.isPartiallyEqualis whatexactQuery: 'partial'means. The current route carries whatever else is in the URL; a link declaring?tab=generalhas to stay highlighted through a search term and a page number. Keys the route adds are forgiven, keys the link declares are not — the asymmetry that makes it "partial" rather than "subset either way".mapContentNavigationItemrenamestitletolabelandpathtotofor every entry in a@nuxt/contenttree. Get it wrong and the sidebar is a column of blank, unclickable rows.All three reach 100% of statements and branches. Every branch is mutation-checked: forgiving removed keys as well as added ones, dropping the
isConnectedguard, narrowing the scrollbar check to one axis, removing thedeeplimit, and four more each redden a named test.Not covered, deliberately
ai.ts— streaming-state predicates, visible only inside a chat UI.virtualizer.ts— 71%, and what is left is measurement arithmetic.editor.ts— 12% of 587 lines, a slice of its own if it is worth one at all.Gate
lint,typecheck, 336 test files / 7676 tests, and coverage 74.31 → 74.76% statements, 71.53 → 71.86 branches.Checklist