test(web): work toward 100% mutation coverage - #1255
Merged
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Mearman
force-pushed
the
feat/100-percent-mutation-web
branch
from
September 13, 2026 14:29
a410718 to
1eaa4cf
Compare
Mearman
force-pushed
the
feat/100-percent-mutation-web
branch
3 times, most recently
from
September 14, 2026 09:17
a070fed to
cbf3b35
Compare
Mearman
marked this pull request as ready for review
September 14, 2026 13:29
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
….ts and workers/**
tsconfig.json (the main app program) deliberately excludes src/rpc/router.ts and every
src/workers/**/*.ts file, since they belong to tsconfig.worker.json's own DOM-vs-WebWorker lib
split instead. Stryker's typescript-checker plugin requires every mutated file to belong to the
one program its tsconfigFile resolves, so pointing it at tsconfig.json crashed outright the
moment a mutant landed inside router.ts or the worker entry point ("no watcher is registered for
it"). tsconfig.stryker.json is a checker-only program: the same include as tsconfig.json but
without the router.ts/workers exclusions, with DOM and WebWorker unioned (skipLibCheck makes the
pair compile together) so both halves of the app typecheck under the one program the checker
needs.
inferFormatFromFilename and relativeTime had no unit coverage at all despite being pure, easily-tested functions -- every extension/alias mapping, the lowercase-before-match step, the dotfile and no-extension edge cases, and each relativeTime unit boundary (minute/hour/day, floored not rounded) are now exercised directly.
…icker's accept extension
String.split('.').pop() can never return undefined for any input, including a string with no '.'
at all -- split always returns at least one element -- so the '?? "bin"' fallback in
createNativeFileAccess's saveFile was dead code with no test able to reach it. Removed the guard
and added full unit coverage for all three file-access adapters (createFileAccess's native/
fallback selection, the fallback picker's file-chosen/dismissed/accept-attribute paths and its
Blob-URL download-anchor save, and the native picker's open/save flows including the
AbortError-vs-real-failure branches and the accept-extension derivation this fix touches).
Neither src/db/dexie.ts nor src/hooks/useRecentFiles.ts had any unit coverage -- jsdom implements no IndexedDB of its own, so nothing could construct the Dexie instance at module load without one. Adds fake-indexeddb (installed globally in the unit project's test setup, ahead of any test's own import of the db module) and exercises the database's own table schema plus recordRecentFile's 20-entry FIFO eviction and removeRecentFile.
…nt converter None of src/hooks/**'s useMutation/useQuery wrappers around getRpcClient(), nor workerDocumentConverter's own convertViaWorker, had any unit coverage. Adds a small, dependency-free render harness (mounting a hook inside a real jsdom tree via react-dom/client and a fresh QueryClientProvider, the same approach src/ui/contentBlocks.test.tsx already established for component-level tests) and a fully-typed mock RPC client fixture (one vi.fn() per router procedure), then uses both to exercise useConversions, useDocumentFormats, useReadMetadata, useWriteMetadata, useExtractSourceFonts, useReadContent, useRestoreContent, useReadOdb, useOdmRender, useConvert, the five useEditorSession mutations, usePdfObjectUrl's blob-URL lifecycle, and convertViaWorker's own field-narrowing of its RPC call.
contentInspectResult, useReadContent, useInspectPdfBytes, and useInspectDocument (src/hooks/ useInspect.ts) had no coverage -- exercises the pure content-backed result builder, the content.read/pdf.inspect RPC calls, and useInspectDocument's own branch between inspecting PDF bytes directly versus converting a non-PDF source to PDF first and carrying the conversion's own diagnostics through.
router.test.ts already covered normalizeContentForSource and the editor-session helper functions directly, but none of router.ts's actual exported procedures (formats.list/listConversions, convert, content.read/restore, metadata.read/write, fonts.extractSourceFonts, pdf.inspect, and the full editor.open/setParagraphText/addParagraph/removeParagraph/save lifecycle including its unknown-session-id error path) had ever been called through oRPC's own dispatch. Uses @orpc/ server's call() to invoke each procedure directly against real markdown/docx fixtures. Forced onto vitest's node environment: jsdom's own TextEncoder constructs its Uint8Array in a different realm than the bare Uint8Array a z.instanceof(Uint8Array) input schema checks against under jsdom, which otherwise rejects every real byte payload as "expected Uint8Array, received Uint8Array".
Adds a Mantine-aware mount harness (mountWithMantine) and a real DiagnosticsPanel test suite, asserting on the Spoiler wrapper's own class marker rather than its "Show N more" label text: jsdom has no layout engine, so Spoiler's internal measured-height-vs-maxHeight comparison can never observe a real overflow and the label never renders regardless of item count. Stubs window.matchMedia and ResizeObserver in the shared jsdom test setup, guarded on `typeof window` since router.procedures.test.ts forces a node environment for the same file. Both APIs are called unconditionally by MantineProvider/Spoiler on mount, so any test that mounts a Mantine component needs them regardless of what it actually exercises. Restates vitest.mutation.config.ts's own setupFiles key, dropped by the same object-literal override that already restates environment: "jsdom", since fake-indexeddb needs to install before dexie.ts's module-scope Dexie construction runs.
main.tsx called its own root-mounting logic unconditionally at module scope, so the only way to exercise the missing-#root failure path was to import main.tsx itself -- which immediately mounts the real App against whatever #root element the test environment's own document happens to have. Moving the logic into mountApp.tsx, parameterised on the target Document, lets mountApp.test.tsx drive both branches directly against a throwaway jsdom Document, with createRoot mocked so the real router/worker stack is never pulled in. tsconfig.node.json's own program never included src/vite-env.d.ts, so the ambient __APP_COMMIT_SHA__ family of build-time globals were invisible whenever a test transitively imported far enough into the app (App -> router -> routeTree.gen -> every route, including -Sidebar.tsx, which reads them) to pull those files into that program. mountApp.test.tsx's own import chain is the first test to reach that deep, surfacing the gap.
Adds direct coverage for the three cases contentBlocks.test.tsx never exercised: an <annotation> element skipped along with its children, a cdata/comment/declaration/pi node producing no displayable content at all, and interleaved text/skip siblings rendering in order. Removes the redundant containerRef.current null check in MathMlView's effect: the ref is attached to an unconditionally rendered element of the same component instance, and React attaches refs during commit, strictly before a passive effect can observe them, so the guard could never genuinely take its true branch.
…ror's Error/non-Error split notifySuccess picks colour, title suffix, message, and autoClose entirely off whether any diagnostic is warning-severity and how many there are; notifyError reads .message off a real Error but stringifies anything else thrown. Neither had a test before this.
…ontract Asserts the empty-mailbox default, a plain set-then-take round trip, that a take clears the entry so a second take sees nothing, and that a later set overwrites an earlier entry nobody ever took.
Asserts no Tree root renders for a value with no browsable children (an empty object, or a primitive), and that one does once the value has at least one array/object entry to browse.
… SheetPreview Adds a shared src/test/fixtures.ts (a real DocumentTreeJson and a page size, built once outside src/ui/** so no UI test needs to import documents.js's conversion functions directly and trip the package's own import-boundary lint rule). InspectPanel: loading/error/empty branches, content-backed summary + structure tree, pdf-backed page count (singular vs plural), item-kind table, and conditional title/producer lines. FormulaPreview / WordProcessingPreview: the shared loading/error/ no-content/wrong-kind-of-document branches every format-specific preview repeats, plus each one's own real rendering path (MathML for a formula document, section blocks for a wordprocessing one). SheetPreview: single-vs-multiple-sheet SegmentedControl visibility, hidden row/column filtering, index-based ordering independent of array position, the empty-sheet fallback for no visible rows/columns, and a cell's own displayText rendering.
…ering Covers the presentation-vs-drawing content split (slides vs pages), single-vs-multiple-slide SegmentedControl visibility, every vector kind (rect, ellipse, line, path with line/cubic segments and open vs closed subpaths), solid/dashed/dotted/double stroke rendering (the double case simulated as a thick underlay plus a thin gap overlay, gap colour falling back to white when the shape has no fill), rotation transforms, paintOrder-driven ordering with an unset order sorting last, and a shape's own fontScale/lineSpacingReduction CSS derivation.
…malisation Mocks @mantine/dropzone's own Dropzone with a plain button exposing onDrop/onClick directly, since FileUpload's own logic (reading a dropped file's bytes, recording it when its extension resolves to a known format, opening the native picker when supported, normalising a single accept extension string into the array Dropzone expects) is what this package's mutate glob covers -- not the third-party drag-and-drop machinery Dropzone itself provides. Covers: file-present vs empty state (icon, name, hint visibility), loading/disabled passthrough, accept normalisation (string vs array, undefined), native-picker-driven onClick/activateOnClick wiring, a dropped file with no entries, and an unrecognised extension being handed to onFile without being recorded.
…ormatting Mocks useRecentFiles/removeRecentFile, useNavigate, notifyError, and setPendingReopen directly rather than exercising real IndexedDB and routing, since those are already covered by their own dedicated test suites -- this file's own logic is the permission-then-read-then- navigate chain, byte-size formatting thresholds, and the disabled/ unrecognised-format guards around it. Covers: the loading/empty/populated list states, B/KB/MB size formatting boundaries, the reopen action disabled with no handle, remove-by-id, a granted-on-first-query reopen, a granted-only-after request, a denied permission (notifies, never navigates), a read failure (notifies with the thrown error), and an unrecognised stored format (does nothing, silently).
Covers every recognised paragraph styleId (heading-1..6, quote, code-block, horizontal-rule, and the plain-paragraph fallback), image and table block delegation (the latter recursing back through this same markdown pipeline for cell content), and the list-grouping behaviour specific to this component: consecutive ordered/bullet runs collapse into one <ol>/<ul>, a type change between adjacent siblings splits into two lists, a deeper-level item nests inside its parent <li>, and a non-list paragraph interrupting a run starts a fresh list group afterward rather than merging with it.
Excludes *.test.ts(x) from the router plugin's route-tree scan (routeFileIgnorePattern) so a route's own unit test file doesn't itself get treated as an undeclared route -- the existing dash-prefix convention in this directory is for genuine non-route support files (-Sidebar.tsx), not a fit for a test file that belongs named like every other test in the package.
…e, testable lookups activeColorSchemeOption/nextColorSchemeOption/optionAt were inline RootLayout logic reachable only by mounting the full AppShell inside a real router and Mantine tree. Extracted as plain functions over a string value, __root.test.ts now drives every branch directly: optionAt's out-of-range throw (never reachable through RootLayout's own two call sites, since the modulo arithmetic guarantees a valid index, but a real invariant worth asserting explicitly rather than papering over with a silent fallback), an unrecognised current value falling back to the first option, and the wrap-around from the last option back to the first.
The router plugin's autoCodeSplitting rewrites every real route file's component behind a dynamic import, entirely independent of whether anything actually imports through the generated routeTree.gen.ts -- the transform keys off the route file's own path. A route-level unit test necessarily imports a route file directly (there is no other way to reach Route.options.component), so mounting it genuinely suspended waiting on a chunk vitest has no build pipeline reason to ever resolve quickly, and the very first such mount in a whole run could take several real seconds. Gated off under mode "test" the same way `base` above is already gated on `command`: a production bundle-size optimisation has no business affecting whether or how fast a test can render a route's component. Confirmed the real build still code-splits every route into its own chunk exactly as before.
…at guard Mocks the RPC client and FileUpload directly, exercising FontsPage's own composition: a recognised format triggers extractSourceFonts and renders each family with its bold/italic flags, an empty result shows the no-embedded-fonts message, an unrecognised extension neither calls extraction nor loses the alert, and a rejected extraction leaves no font table behind.
…cases formatLeaf's two truncation checks (the raw-string cap at 102 characters, the generic cap at 100) were only ever tested well above or well below each threshold, leaving both > comparisons free to become >= without any test noticing. Added exact-boundary fixtures for both the string and non-string (bigint) paths. kindSuffix's own early return for a non-plain-object value was never reached at all -- every existing array-item fixture was either a leaf or a plain object, never a nested array. Its kind !== string branch was similarly untested, since the one array-of-objects fixture always carried a genuine string kind. Added a nested-array item, an object item with a non-string kind, and an object item with no kind field.
…lob contents input.type was set but never checked by any test; the change listener's once: true option was passed but no test verified it was actually attached that way, and saveFile's Blob was only ever exercised indirectly through a mocked createObjectURL that ignores its own argument. Checked input.type directly, checked addEventListener's own call arguments for the change listener, and read the Blob object itself back off createObjectURL's mock call to assert its real size and type.
…tent
toContain("pages")/toContain("image")/toContain("1") loose substring
checks can pass even when the item-kind table renders nothing, since
the structure tree below separately renders its own "pages [0]" node
and formatVersion text containing the same substrings. Assert against
the mounted DOM's actual table rows and the precise pluralised count
string instead.
…M shape Add a not-loading counterpart to the existing loading-overlay assertion so an inverted loading check is genuinely covered both ways. Replace the ordered-list test's loose toContain checks with real querySelectorAll assertions on <ol>/<ul>/<li> counts and text, since substring matching cannot tell a genuine single <ol> apart from stray nesting or duplicate list rendering.
…ll edge cases mountWithClassName's own appendChild/cleanup contract had no dedicated test at all -- the one caller (FileUpload.css.test.ts) only reads getComputedStyle, which jsdom resolves against a class selector whether or not the element is actually connected to the document, so neither appendChild being skipped nor cleanup being gutted changed that test's outcome. Added a direct test asserting document.body actually contains the element, and that cleanup removes it again. matchMedia's dispatchEvent stub return value (true, per the real EventTarget contract) was never asserted. cloneAndCollectTransferableBuffers' own array loop bound was checked with a Proxy that records every index actually read, proving it never reaches one past the array's own length -- reading one past the end is otherwise silently harmless (an out-of-bounds array read is just undefined), so no value-based assertion on the function's output could have told the two apart. stripMathMlNamespace's own ternary was provably redundant: slice(-1 + 1) already equals slice(0), the whole original string, for a tag with no namespace prefix -- the same value the "no colon" branch was returning by name. Collapsed to the unconditional slice call.
…memoise the file access port handleClick is only ever wired up as Dropzone's onClick when supportsNativePicker() already holds. Its own internal check of the same condition could never see it fail. Also covers createFileAccess() being recreated on every render. useMemo's empty dependency array is now verified stable across re-renders.
…a redundant length guard An absent styleId no longer falls back to an empty string before the heading-pattern match. It short-circuits to no match directly, since any non-matching string produced the same outcome. Also drops the length > 0 check before recursing into a list item children. Mapping an empty array already renders nothing, so the guard was already a no-op.
… handle check as disabled One boolean now feeds both the disabled state and the tooltip label. Previously each read record.handle independently with an inverted comparison. A record without a handle disables the button but never renders its own tooltip text. Only the disabled assertions already covering both cases could ever observe either check.
…its empty-sheets branch A number cell already produces the same right-aligned class a percentage or currency cell does. Two new tests render each kind on its own, so a percentage or currency check going missing. That would surface as a different alignment class, not just a match already satisfied elsewhere. Also drops the sheets.length > 0 branch from the active-index clamp: clamping against a length of 0 already yields an out-of-range index that reads back as undefined the same as the 0 this branch special-cased to.
…des guards An absent fontScale templated to the invalid CSS length "undefinedem", which a browser's CSSOM (jsdom included) already rejects as a no-op rather than ever writing -- the same rendered outcome as the value being undefined, which React also drops from a style object. Also drops the slides.length > 0 branch from the active-index clamp for the identical reason already applied to SheetPreview's own clampedIndex: clamping against a length of 0 already yields an out-of-range index that reads back as undefined regardless.
…zy initializer useMemo(() => createFileAccess(), []) left an empty dependency array whose own mutation to a constant, equal-length array survived: useMemo compares dependencies element by element against their previous values, never the array's own identity or length, so a hardcoded literal in that position memoises exactly as stably as an empty array does. useState's lazy initializer runs exactly once on mount by React's own contract, with no dependency array for a mutation to tamper with.
…changing deps array useLiveQuery's deps feed a plain React useMemo internally. A fresh empty array literal on every call memoises identically to any other constant-content array, since useMemo compares elements against their previous values, never the array's own identity or length. A module-scope constant makes that array's own contents a static, import-time mutation instead of a per-render one -- exactly the class ignoreStatic already excludes workspace-wide. Also adds a call-count assertion on bulkDelete: the existing eviction tests already prove the final row count converges either way once staleCount reaches zero, which is why they could not previously tell "the query was skipped" apart from "the query ran and correctly found nothing stale".
buildDocumentBytes, the tree-based docx writer every other test in this suite goes through, always synthesises a bullet-format numFmt regardless of the source ContentListMembership's own format (#1273). No bytes it produces can exercise the ordered-list branch of router.ts's own numId resolution. Building genuine bytes directly through ooxml.js's own flat writer instead gives a numbering.xml whose numId is unambiguously decimal, bypassing the writer bug rather than working around it.
…s bullet A none numFmt (ECMA-376's own spelling for an invisible marker) needs its own real numbering.xml fixture, distinct from the decimal one already covering the ordered branch, to prove it resolves as bullet rather than merely not being decimal. Also covers normalizeMarkdownParagraph's own numId fallback: markdown-codec only ever mints a numId matching its own grammar, so the fallback is unreachable through the real pipeline, exercised directly the same way the docx/odt fallbacks above already are.
…hable fallback split(...).pop() can never actually return undefined (splitting a string always yields an array of at least one element), so the `?? filename` fallback around it was dead code with no test path that could ever exercise it. Replaced with a last-separator lastIndexOf/slice, whose own -1 "not found" case is a real, already-tested branch (a filename with no directory separator at all).
…ngth estimate asset.base64 is always produced by documents.js's own bytesToBase64 encoder (readPdf's image extraction is the only producer of a LayoutImageAsset), which pads every output to a multiple of 4 characters -- a real base64 encoding's own length invariant. length * 3 / 4 is therefore already exactly integral for every value this ever receives, so Math.ceil around it could only ever be a no-op with no input that could distinguish it from floor or round.
… is present inferFormatFromFilename's own leading-dot guard (dotIndex <= 0) only ever saw a relative index of 0 because the existing tests never paired a directory separator with a dotfile-style final segment. A filename like "notes/.docx" exercised the guard correctly, but nothing proved the last-separator computation (or the slice built from it) was what kept that index at 0 rather than some other, coincidentally-identical behaviour.
A genuine full, non-incremental Stryker run scored 100.00% (1441 killed, 13 timeout, 0 survived, 0 no-coverage of 1454 valid mutants), so the break threshold is pinned at the maximum with no timeout-share margin below it -- there is nowhere left to flap down to.
…nd the literal banned phrase eslint-config 2.12.1 added a workspace-wide no-warning-comments ban on the literal substring "stryker disable" anywhere in a comment, to catch real mutant-suppression comments. The break-threshold justification comment used that exact phrase only to describe that none exist in this package, tripping the ban as a false positive. Reworded to state the same fact without the banned substring.
vi.fn() with no generic infers a return type of any, which @typescript-eslint/strict-void-return (added in eslint-config 2.12.1) now rejects wherever a listener parameter's own type declares void. Pinning each mock's generic to a void-returning signature matches the real addEventListener/removeEventListener listener type these tests exercise.
…ad of Object.assign exadev/no-object-assign (added in eslint-config 2.12.1) bans Object.assign outright, since its type declarations never check a source object's properties against the target's. Object.defineProperties keeps the real File instance's prototype chain intact while adding path/handle/arrayBuffer -- unlike an object spread, which would drop File's own name/size/type (they live on the prototype as accessors, not as own enumerable properties, so spreading a File loses them silently). Also types the onFile fallback mock as void-returning for the same strict-void-return reason as the matchMedia listener mocks.
An arrow function `(resolve) => setTimeout(resolve, 0)` returns setTimeout's own Timeout handle, which @typescript-eslint/strict-void-return (added in eslint-config 2.12.1) rejects for a Promise executor's void-typed parameter. A block body discards the return value explicitly.
Mearman
force-pushed
the
feat/100-percent-mutation-web
branch
from
September 14, 2026 18:08
8e4f837 to
da188c2
Compare
Rebasing onto main pulled in a newer ooxml.js release without updating this package's own exact pin, so syncpack's DiffersToLocal check failed CI. Every internal dependency in this workspace is pinned to an exact version rather than a range, so the pin has to track the sibling's released version directly.
…bserver's own
The stub previously had no explicit constructor at all, so the test that
constructs it, new ResizeObserver(() => {}), passed an argument a
zero-parameter default constructor doesn't accept. A TypeScript parameter
property both declares the same callback parameter the real ResizeObserver
constructor requires and avoids the two alternatives tried first: a
written-out no-op statement (void callback;) and an empty constructor
body each leave the field's storage indistinguishable from not storing it
at all, which Stryker's mutation testing confirmed empirically as an
unkillable survived mutant either way. A parameter property's implicit
assignment is compiler-emitted, not literal source, so there is no
explicit statement for Stryker's instrumentation to mutate in the first
place.
Contributor
|
🎉 This PR is included in version 2.11.23 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
1 participant
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.
Fixes the Stryker typescript-checker crash for this package (router.ts and workers/** belong to tsconfig.worker.json, not the app tsconfig.json Stryker's checker was pointed at -- neither program alone covers everything the mutate glob touches) and adds real unit coverage across the previously-untested pure helpers, file-access adapters, the IndexedDB-backed recent-files store, every RPC-client-wrapping hook, every format-neutral preview component, and several route/UI files.
Progress so far, not yet complete:
?? "bin"fallback in the native save pickerautoCodeSplittingrewrites every route file's component behind a dynamic import regardless of whether anything goes through the generated route tree, so mounting any route'sRoute.options.componentdirectly in a test genuinely suspended on the first render. Gated off under vitest's own test mode, the same way this config already gatesbaseon the build/serve command -- confirmed the real production build still code-splits every route exactly as before.Remaining gap: src/rpc/router.ts's harder procedures (fonts.describe, odb.read, odm.render, non-markdown editor.save) are still untested; most of src/routes/** (recent.tsx, odb.tsx, inspect.tsx, package.tsx, odm.tsx, metadata.tsx, editors.tsx, convert.tsx, -Sidebar.tsx) has no unit tests yet. A first real Stryker baseline run is in progress (partial data so far: roughly 1100+/2870 mutants tested, ~50 survived) but has not completed within this session -- the package's real size (3000+ mutants across 81 mutated files) combined with heavy contention on the shared machine this ran on means a full run takes upward of an hour. Work continues on this branch.
No Stryker disable comments anywhere in the package.