test(markdown-codec): work toward 100% mutation score - #1265
Draft
Mearman wants to merge 18 commits into
Draft
Conversation
…ble undefined branch mintListNumId now has tests pinning that a bullet mint ignores a supplied start value and that an ordered mint with no start omits the @n suffix entirely, rather than stringifying undefined into it. parseListNumId gains a test for a numId with a numeric suffix on a bullet marker (a shape the regex itself allows, since the suffix isn't gated on type), which the parser must still treat as start: undefined. parseListNumId's own type-narrowing guard dropped its `type === undefined` half: NUMID_PATTERN's second capturing group is a mandatory alternation with no `?`, so a successful match always populates it, and the `type !== "bullet" && type !== "ordered"` half already answers `true` for `undefined` on its own -- the dropped half never distinguished any real input from the other.
…ity guards Adds direct tests for headingStyleId/parseHeadingStyleId: level 0 rejected (a heading style level is always positive), a 400-digit run rejected (it parses to Infinity, which Number.isInteger correctly refuses), and a level past the markdown-reachable 1-6 ceiling still parsed, since ContentDocument is a shared cross-format pivot other producers may carry a deeper heading level through.
…nt keys lowerTable's own column-width arithmetic (contentWidthPt / columnCount) had no test distinguishing it from any other arithmetic on the same two numbers, since the existing test only checked that both columns came out equal to each other. Adds a test with an explicit page size and margins so the expected per-column width is a known, exact number. Also pins that a table cell with no run-level constructs carries no `constructs` key at all, and a column the delimiter row leaves unaligned carries no `alignment` key -- both spread conditionally, and neither had a test checking the key's absence rather than just its rendered content.
…uard clauses No test called matchMathInlineSpan directly before this -- it was only exercised indirectly through the inline parser's own already-real \(...\) input, which never distinguishes the guard's two sub-conditions from each other or from a forced true/false, since a genuine match never needs to fall through to a wrong answer. Pins: a real span; an unterminated \( with no test each individually; and that the closing search starts strictly after the opener, never before it (a preceding, unrelated \) must not be mistaken for the real close).
…er grammar matchFootnoteLabel, matchFootnoteDefinitionMarker, and isValidFootnoteLabel had no test calling them directly -- only src/footnote.test.ts's end-to-end round trips through the whole read/write pipeline, none of which exercises a valid label with no following colon (a reference, not a definition) or text that never matches the label grammar at all.
…ight axes The only existing coverage (lower.test.ts's 1x1 PNG fixture) happens to carry the same value on both axes, so a widthPt/heightPt swap or a wrong operator on either axis produces no observable difference. Adds a real 300x100 PNG fixture and checks each axis converts its own pixel dimension to points independently.
…mages Every existing image emit test supplied altText, so the ?? "" fallback for a ContentImageBlock with none at all was never exercised.
…arkdownInlineNode Neither predicate had a single test or internal caller before this -- they were dead code as far as this package's own test suite could tell, even though both are part of the module's public surface. Pins block vs. inline classification for a representative of each side, plus every real block node type named in BLOCK_NODE_TYPES individually.
…s/source table
readMarkdown's own definitions/source splice special-cased "neither table
applies" to return assembleTree's result unchanged, rather than spreading
it. The spread was already a no-op in that case -- spreading undefined,
or an absent optional key, adds nothing -- so the shortcut bought only an
object reference identity DocumentTree's own contract never promises, at
the cost of a branch no value-level assertion could ever tell apart from
always spreading. Also drops the `assembled.source ?? {}` fallback the
frontmatter splice used: spreading `undefined` directly is exactly as
inert as spreading `{}`, so the fallback never changed the result either.
Extends package.test.ts's coverage of the write side to match: a titleless
link reference definition (no title key on the rendered entry, and no
trailing title clause in the written text), two definitions joined by a
real newline rather than a coincidentally-equal separator, and a
definitions-only document (empty body) rendering the definitions bare
with no leading blank line.
lineIsBlank's own class-field default (false) could never be observed to differ: the constructor unconditionally calls findNextNonspace() immediately afterward, which always assigns the real value before any getter can read it. Dropped the initializer (definite-assignment `!:` instead) rather than leave a default no test could ever tell from any other value. advance()'s early return at end of line is the same shape: MarkdownScanCursor.next() is already a side-effect-free no-op once rawOffset reaches the source length, so looping the remaining count down regardless produces the identical end state as returning early. Dropped the guard. Adds direct LineCursor tests for blank-line detection (empty and whitespace-only lines, and a non-blank one), which the package had none of before this -- the class was only ever exercised indirectly through src/block/block.ts's own parsing.
…t operations InlineNode had no test of its own before this: appendChild, unlink, and insertAfter were only ever exercised indirectly through the inline parser's own emphasis/link resolution, which never isolates a single operation's own effect on the surrounding chain. Pins each field's default for a node kind that never sets it, appendChild's ordering, unlink's neighbour re-linking (mid-chain and at either end), and insertAfter's own three distinct behaviors: splicing in a fresh node, detaching a node from its OLD chain before relinking it into a new one, and updating (or correctly leaving alone) the parent's own lastChild depending on whether the insertion lands at the end.
…htness logic isBulletMarker/isOrderedDelimiter narrowed a regex match's own capture group to a literal type, but both patterns' character classes already guarantee the value (BULLET_MARKER_PATTERN is exactly `[*+-]`, ORDERED_MARKER_PATTERN's second group is exactly `[.)]`) -- neither predicate's "not a member" branch is reachable from a real match, so both became a plain cast at their one call site each, with a comment stating why it's safe. parseListMarker's own trailing-spaces scan drops three more branches that turned out to be fully compensated for downstream rather than genuinely decisive: the do-while's own code-indent cap (the reset branch already re-derives the item's content indent from scratch whenever the count exceeds it, so the cap only changed how far the loop itself walked, never the returned value or the cursor position it leaves behind), the `followingSpaces < 1` disjunct (the do-while's own do-first structure means that can only ever be true when startsBlank is also true, so it was never an independent second condition), and the reset branch's own `if (line.peek() === " ")` guard on its own follow-up advance (the marker-follows-by check earlier in the function already guarantees the character there is a space/tab/EOL, and advancing past EOL is a no-op, so the guard's own false side is equally unreachable). Adds src/block/list.test.ts: direct coverage of listsMatch's own three fields (type/delimiter/bulletChar) and of finalizeListTightness's lastLineChecked memoisation actually setting the flag on both the descend-further and stop-and-return-false paths, neither of which any existing test observed directly.
…testable
Four scan loops (matchLinkLabel, parseLinkDestination's angle-bracketed
form, parseLinkTitle, skipInlineWhitespace) bounded themselves with
`index < text.length`, which turned out to be indistinguishable from
`index <= text.length` for every one of them: text.charAt(index) already
returns "" one index past the end, and none of these loops' own character
comparisons ever match "" either, so the one extra boundary iteration
always falls through to the identical exit path regardless of which
comparison guards it. Rewritten as `text.charAt(index) !== ""` instead --
exactly the same boundary for every real index, but one whose own
mutation (the operator, or the "" literal) is now actually reachable by a
test rather than always landing on the same fallthrough either way.
parseLinkTitle's own `closer === undefined` guard is the same shape: when
`opener` isn't one of TITLE_DELIMITERS' own three keys, `char === closer`
can never match a real character, and TITLE_DELIMITERS' own mapping means
`opener` is only ever "(" when closer IS defined -- so the loop already
scans to the end and returns undefined regardless, and the guard bought
nothing an early return wouldn't have. Dropped in favour of a comment
recording why.
Adds direct tests for four scenarios nothing exercised before: a start
that isn't "[" with a ']' reachable later (matchLinkLabel), an unescaped
nested '<' with no line ending (parseLinkDestination's bracketed form), a
trailing unescapable backslash treated as a literal character rather than
the start of a truncated escape (parseLinkDestination's bare form), and
isBlankRemainderOfLine's own four cases (nothing exercised it at all
before this) including reaching the true end of the text.
…om MarkdownScanCursor atEnd()'s own `pendingTabColumns === 0` half was never independent of the rawOffset check beside it: rawOffset only advances past a tab once every one of its columns is spent (next()'s own tab branch), so rawOffset can never reach source.length while a tab is still mid-expansion. Checking rawOffset alone already answers the same question. peek() dropped both its `pendingTabColumns > 0` branch and its own `rawOffset >= source.length` guard: while a tab is mid-expansion, rawOffset still points AT that tab character (the same invariant atEnd relies on), so the plain read below already finds '\t' and returns the correct synthetic space through its own tab branch; and past the end of input, a string index in JS is already `undefined` on its own, which matches every comparison below it and falls out the far end as `undefined` regardless. Both "extra" branches produced the identical answer the plain read below them already gives, on every reachable input. next()'s own end-of-input guard is NOT the same shape and stays: skipping it would still return the correct `undefined`, but it would also mutate rawOffset/columnNumber for a character that was never really there, corrupting the cursor's own state on every subsequent call. Added a test pinning that calling next() repeatedly past the end is idempotent. Adds direct coverage for what was previously untested at all: peek()'s own '\r' normalisation and true-end-of-input case, and peekRaw() actually slicing (a same-length fixture had let it read as `this.source` with the slice call itself elided).
… HTML recogniser
matchHtmlTag's own text.charAt(start) !== "<" guard and
matchHtmlBlockStart's own !line.startsWith("<") guard both duplicated a
fact their real regexes already enforce: every alternative in
HTML_TAG_PATTERN, and every real entry in HTML_BLOCK_START_PATTERNS
(types 1-7), is itself anchored at `^` and begins with a literal '<' in
its own source -- so a string that doesn't open with '<' already fails
every one of them on its own, and the dedicated guard could only ever
agree with what the pattern match was already going to answer.
… and canContain BlockNode's replaceWith/unlink and the module-level canContain had no test of their own before this. Pins each mutable field's own empty-string default (infoString/literal/headerLine/footnoteLabel), replaceWith/unlink both correctly no-op-ing when the node they're called on isn't actually present in its own parent's children array (an inconsistent state a wrong `index !== -1` check would otherwise splice(-1, 1) against -- deleting the parent's LAST child instead of nothing), and every one of canContain's own per-parent-kind branches, including the two restrictions specific to a footnote definition.
…wn out-of-range "" splitTableRow's own scan loop and its backslash-pairing check, and endsWithUnescapedPipe's own trailing-backslash count, each paired a length-based bound with a character comparison that can never match "" -- so once the length bound would have stopped the loop, the character check was already going to fail on its own the very next read, on every reachable input. Restated the two loop bounds as `charAt(...) !== ""` (the same boundary, spelled as the check that's actually reachable by a test) and dropped endsWithUnescapedPipe's bound entirely, since charAt of a negative index is already "" with no separate arithmetic needed to say so. parseTableDelimiterRow's own `cells.length === 0` guard is dead for a different reason: splitTableRow always pushes its own trailing `current.trim()` unconditionally, even over empty input, so it can never actually return an empty array. Adds real coverage for what these bounds were guarding in practice: leading/trailing whitespace trimmed before either pipe is read, a leading pipe stripped independently of a trailing one (and vice versa), a lone trailing backslash with nothing to escape treated as a literal character, and endsWithUnescapedPipe's own odd/even backslash-run counting through three and four consecutive trailing backslashes, not just one.
…oint-boundary coverage matchEntity's own '&'-prefix guard is the same redundant shape already fixed for matchHtmlTag/matchHtmlBlockStart: ENTITY_PATTERN's own source is anchored at `^&`, so a slice that doesn't open with '&' can never match regardless. unescapeString's own "neither backslash nor '&' at all" fast path is provably a pure optimisation too: for a string with neither, the loop it skips never takes the backslash/entity branches either, so it does nothing but reconstruct the identical string one character at a time -- same output, more work, never a different result. Its own loop bound gets the same charAt(index) !== "" restatement already applied elsewhere in this codec, for the same reason. Adds direct tests for codepointToString's own three boundaries (U+0000, the maximum codepoint, and the low/high surrogate range) that nothing exercised before -- each just below, at, and just past its own edge, so each comparison's own direction and operator is pinned rather than only its "obviously in range" and "obviously out of range" interior points.
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.
Summary
Working through markdown-codec's survived/no-coverage mutants toward a genuine 100% Stryker mutation score, per the same pattern already applied to other packages in this workspace (archive-codec, byte-codec, document-compute.js, excel-number-format, pdf-raster-cpu, ...).
Measured baseline: 76.32% of 4529 valid mutants, timeout share 4.9% (
breakThreshold: 71instryker.config.ts).Progress so far (small/foundation modules):
shared/list-id.ts,shared/style-constants.ts,lower/table.ts,inline/math.ts,inline/footnote.ts,lower/image.ts,emit/image.ts(viaemit.test.ts),ast/ast.ts,read.ts/write.ts(viapackage.test.ts),block/line.ts.Two genuinely-equivalent mutants were eliminated by restructuring rather than adding an unkillable test:
read.ts: dropped a reference-identity-only shortcut inreadMarkdown's definitions/source splice, and a redundant?? {}fallback that spreadingundefinedalready made a no-op.block/line.ts: droppedlineIsBlank's dead class-field default (always overwritten by the constructor's ownfindNextNonspace()call before any read) andadvance()'s early-return (already a no-op past end of input).No
// Stryker disablecomments anywhere -- confirmed viagrep -r "Stryker disable" src.This is a large package (block.ts, emit.ts, inline.ts, lower.ts, image/image.ts, the two html-table.ts pairs, html/render.ts, and gfm-autolink.ts/chars.ts alone account for the majority of the ~1000 remaining survived/no-coverage mutants) -- still in progress, left as draft until the score is genuinely verified at 100.
Test plan
pnpm --dir packages/markdown-codec typecheckpnpm --dir packages/markdown-codec lintpnpm --dir packages/markdown-codec exec stryker run stryker.config.tsat 100%breakThresholdraised to 100 once verified