test(ooxml.js): work toward a 100% mutation score - #1259
Draft
Mearman wants to merge 41 commits into
Draft
Conversation
…code buffer sizing Adds direct coverage for bytesToBase64/base64ToBytes across every input-length remainder (0, 1, 2 bytes past a full 3-byte group), the invalid-base64 throw for each of the two positions a malformed character can occupy in a 4-character group, and whitespace stripping before decode. base64ToBytes now builds its output as a plain number[] converted via Uint8Array.from rather than pre-sizing a Uint8Array from a `len * 3 / 4` estimate: that estimate is only ever an upper bound, so any formula that never under-counts is behaviourally identical to any other once the result is trimmed to its real length -- removing the sizing arithmetic as an AST node rather than leaving an unobservable estimate for a mutation to hide behind.
…lder scaffolding
Adds direct coverage for buildXml across every XmlNode variant (text,
comment, cdata, pi, declaration, attribute-less and attributed
elements, nested children, multiple root nodes) and for
assertBuiltString's own throw, extracted from buildXml so the "did the
builder return a string" guard is directly testable with a non-string
literal rather than left uncovered forever (XMLBuilder, given this
module's fixed options, never actually returns anything else).
Simplifies two spots verified directly against fast-xml-parser to be
unobservable: a processing instruction's and a declaration's own child
array is never rendered by the builder under this configuration (`{
"?custom": [{ "#text": "x" }] }` and `{ "?custom": [] }` build to the
byte-identical `<?custom?>`), so neither carries a value the builder
ever reads; and an element's own `:@` attributes object is set
unconditionally rather than gated on whether any attribute exists,
since an empty `:@": {}` builds identically to the key being absent
and parseAttributes already reads both back to the same empty array.
Exports and directly unit-tests every one of parseXml's own structural guards and error paths (isRecord, isUnknownArray, asString, parseNodes, parseNode, parseAttributes, scalarText) against synthetic fast-xml-parser-shaped input: a node that is not an object, a node with no tag key or more than one, an attribute value or scalar-text wrapper of the wrong shape. Real fast-xml-parser output never produces these malformed shapes, so none of these branches was ever exercised through parseXml's own public entry point alone.
…variant Adds direct coverage for isXmlNode's own structural guard across non-record inputs (null, an array, a primitive -- each a distinct branch of typeof/null/Array.isArray that real Zod-validated input never separately exercises), every XmlNode variant's own required fields, malformed attribute entries, and a recursive check that a child element's own children are validated the same way rather than only its own direct fields.
…edundant bounds check Adds direct coverage, via packageFromEntries's own xml/binary classification, for a UTF-8 BOM prefix (alone and combined with leading whitespace), every individual whitespace byte the format permits, a run of several in a row, an all-whitespace part with no non-whitespace byte at all, and a part whose first three bytes only partially match the BOM (isolating each of the three signature bytes' own necessity) -- none of which any existing test exercised. Drops looksLikeXml's own `bytes.length >= 3` BOM guard: it is provably redundant given how out-of-range Uint8Array indexing behaves -- an index at or past a real array's own length always reads `undefined`, which can never equal a real BOM byte, so a short array already fails the byte-by-byte comparison on its own. The main scan loop is likewise rebounded on `bytes[i] !== undefined` rather than a separately tracked `i < bytes.length`, for the identical reason.
…ant bounds check Adds direct coverage for sniffImageFormat across every recognised signature (PNG, JPEG, both GIF header versions), near-miss prefixes that diverge partway through or on the final byte, and SVG detection by its own XML-prolog and bare-root-tag spellings, leading whitespace before either, and the 1024-byte sniff window's own boundary (a real '<svg' tag placed well past the window must not be found there). Drops startsWith's own `bytes.length < signature.length` guard: it is provably redundant given how out-of-range Uint8Array indexing behaves -- an index at or past a real array's own length always reads `undefined`, which can never equal a real signature byte, so a shorter array already fails the byte-by-byte comparison on its own.
Adds direct unit coverage for relsPathFor (a slash-free part path, and a nested one where only the LAST slash may split it) and resolveRelTarget (a package-rooted target, a relative target against both an empty and a real subject directory, a '../' segment popping the enclosing directory, a '.' segment, and a doubled-slash empty segment) -- neither function was reachable from any existing test except through a much larger relationship-resolution fixture that never varied these specific shapes.
… redundant date checks Adds direct coverage for serialToIsoTime/serialToIsoDateTime's own non-finite and negative-serial rejections, and for utcMsOfCalendarDate's own year/month rollover rejections -- including a day value large enough to roll a whole leap year forward, the one shape that makes the year check's own necessity observable (the public isoDateToSerial entry point never passes a day outside 0-99, which alone never triggers it). isoDateOfDayCount now switches on the sign of the offset from the phantom leap day rather than pairing an equality check (excluding day 60 itself) with a separate `<` comparison against the identical threshold: with 60 excluded by the `0` case, the remaining two cases are Math.sign's only other outputs, leaving no inequality boundary for a mutation to hide behind. utcMsOfCalendarDate drops its own third, day-level equality check: Date.UTC(year, month-1, day) maps onto exactly one real calendar date, so whenever a re-read year and month both already match what was asked for, day is necessarily inside that month's own valid range and is therefore already forced to match too (confirmed by exhaustive search over every realistic year/month/day combination) -- a third check here could only ever restate a fact the first two already guarantee.
…space split Adds direct coverage for parseSqref (absent/empty input, a single bare cell, a real span, several ranges, a malformed token skipped among well-formed ones), formatSqrefRange (bare cell vs. row-only vs. column-only vs. full spans), and formatSqref's own join -- none of which this shared helper had a dedicated test file for at all. Simplifies the token split from `/\s+/` to `/\s/`: splitting on each individual whitespace character rather than a run of them only ever inserts extra empty strings between adjacent whitespace characters, which the loop's own `token === ""` skip already discards, so both forms produce the identical final token list regardless of how many consecutive whitespace characters separate two ranges.
… directly Adds a dedicated test file for the xlsx rule-residue helpers: capturing zero, some, and every attribute as unmanaged, and reading residue back for an absent source, a wrong-format source, a source that fails to parse as exactly one element, and one whose tag mismatches the expected rule kind -- none of which had direct coverage before.
Adds a dedicated test file: an absent sharedStrings part reads back as exactly an empty array (not a placeholder value), multi-run <si> entries concatenate in document order, and SharedStringTable assigns sequential indices while deduplicating a value interned twice.
…re no tables A plain property read cannot distinguish a genuinely absent key from one spread on with an explicit undefined value -- both read back as undefined. Adds an Object.hasOwn check alongside the existing toBeUndefined() assertion so readXlsx's own conditional spread is actually exercised, not just its value.
… directly Adds a dedicated test file: a sheet with no table relationship at all reads no definitions, a non-table relationship among several is skipped in favour of the genuine table one, and a table part missing its own name or ref attribute is skipped rather than promoted with a missing field.
…at all Adds a case where neither of an image's own neighbours is a paragraph (two more images either side), which no existing fixture in this file exercised -- every prior case had at least one paragraph candidate, matching or not.
…r patterns' own absent key Adds a "none" w:fill and a "none" w:color case (only "auto" was previously exercised for either), and strengthens the existing single-colour pattern tests with an Object.hasOwn check: a plain toEqual cannot distinguish an omitted foregroundColor/backgroundColor key from one spread on with an explicit undefined value, so a genuinely one-sided pattern read needs the stricter check to prove the other key is truly absent.
…nt, and reply linkage Adds a dedicated test file: threadedCommentId's own uppercase-hex formatting (a counter of 10 exercises the digit-vs-letter distinction 0-9 alone cannot), sequential ids increasing across two separately commented cells (not just within one thread), a reply immediately following its own root with the root's real id as parentId while the root itself carries none, and the threaded-comments root's own declared namespace. Exports threadedCommentId, previously module-private, purely for this direct coverage.
… a dead type-narrowing check Adds a test at exactly the half-point tolerance boundary (not just comfortably inside it), and four tests each isolating one dimension's own necessity in pageSizeToPaperSizeCode's Letter/A4 checks (a width match with a mismatched height, and vice versa, for both page sizes) -- none of which any existing test distinguished from the other. parseUniversalMeasureToPt no longer runs an `amountRaw === undefined || unit === undefined` check after a successful regex match: neither of UNIVERSAL_MEASURE_RE's two capture groups is optional (neither has a trailing `?`), so a successful match always populates both -- TypeScript's own RegExpExecArray typing just cannot express that a specific pattern's own groups are mandatory. Non-null assertions state that directly instead of a runtime check no real regex match can ever fail.
…and numeric level ordering
Adds a w:startOverride whose own ilvl names a level the base
abstractNum never defined (must be skipped, not fabricated), a
declared-namespace assertion for the built w:numbering root, and a
level ordering case proving ilvl sorts numerically ('10' after '2'),
none of which the existing round-trip-only fixtures distinguished from
a passing but coincidentally-correct result.
readEmbeddedOoxmlPayload's outer catch swallows a wrongly-detected flavour's own read failure exactly as gracefully as a genuinely undetected one, so testing hasDocxBody/detectFlavour only through that public entry point cannot tell "correctly found no flavour" apart from "wrongly matched one, then threw reading it" -- both produce the same undefined result. Exports both functions and adds direct coverage: a w:body present/absent, and each of the three entry-part flavours detected (or none) independent of the read that would follow.
…cell ContentSheetCellSchema requires displayText, absent from the plain number-cell literals comments-write.test.ts built by hand -- caught by tsconfig.node.json's own typecheck (which includes test files, unlike the base tsconfig.json a plain tsc run checks). Introduces a numberCell helper that always sets it alongside the numeric value.
…tes.length Uint8Array.prototype.subarray already clamps its end argument to the array's own length, so requesting SVG_SNIFF_WINDOW bytes from a shorter buffer already yields exactly the bytes that exist -- the Math.min was never observably different from omitting it.
…hape A value shaped exactly like a valid element (tag/attributes/children all present) under an unrecognised type name must still fall through to the final `return false` -- nothing previously drove the value into the "element" arm by an unrelated type name alone.
…wards A comment thread with one reply, followed by a second cell's own comment, needs the second root's id to continue at 2 -- a reply-loop increment that ran backwards would instead collide it with the first cell's own root id.
A distractor relationship whose type is not the table relationship type, but whose target happens to be a genuinely well-formed table element (name and ref both present), must still be skipped -- the existing distractor test's target failed the name/ref check anyway, so it could not by itself distinguish the type guard from an absent one.
… guard When indexOf finds no 'T', the date half slices to length iso.length - 1 and the time half to the whole iso.length characters. ISO_DATE_PATTERN and ISO_TIME_PATTERN are anchored to exactly 10 and 8 characters respectively, so matching both at once would need iso.length to be both 11 and 8 -- impossible. With no separator, at least one half always fails to parse, so the existing undefined fallthrough already covers it with no separate check needed.
parseRangeReference("") always returns undefined -- its own
parseCellReference requires at least one letter and one digit, which
an empty string can never supply -- so the loop's existing
`range !== undefined` check already discards an empty token with no
separate skip needed.
fast-xml-parser's own builder ignores the array's content entirely for both the "pi" and "declaration" ordered-node shapes (verified directly against the library), so a fresh per-call [] literal there is a live mutation target with no test able to observe a difference. Hoisting it to one array built once at import time keeps the exact same runtime value while making it a static mutant instead, which the workspace's shared Stryker config already excludes from the valid-mutant count for exactly this reason.
…sPathFor textContent's own cdata half was never exercised by any existing fixture (every one used only <t:text> nodes); adds a mixed text+cdata element proving both node kinds concatenate into one string. relsPathFor's fileName ternary is redundant in the same way its own sibling functions elsewhere in this package already are: slice(-1 + 1) is slice(0), which returns the whole string unchanged -- exactly what a slash-free path needs -- so partPath.slice(lastSlash + 1) alone already covers both cases correctly.
…anch ContentCellFillSchema only ever produces 'solid' or 'pattern' through normal validated input, so the writer's own defensive default branch naming the actual kind was never exercised. Passes a fill shaped like neither, past the type system, and checks the thrown message names it.
Adds: a non-image block sitting beside a genuine Caption-styled paragraph must never gain a caption property of its own (proves the "is this an image" guard actually runs, not just that its outcome happens to match); and a caption with multiple runs must join them with no separator between, which every existing fixture's own single-run captions could never distinguish from any other join string.
…fined level
Object property enumeration hoists canonical non-negative-integer
string keys ('2', '10', ...) into ascending numeric order on its own,
with no sort needed at all -- which is exactly why the existing
'10'/'2' ordering test cannot, by itself, distinguish a real numeric
sort from no sort, or from a broken comparator. Adds a non-canonical
ilvl ('00') and numId ('00') to let a genuine comparator show through,
plus a level whose own value is undefined despite carrying an own key,
proving it is omitted rather than written as a hole in w:abstractNum's
children.
bytes[i + 1]/bytes[i + 2] already read back undefined past the array's own end, and the one use of each not already guarded by its own boundary ternary (the b1 >> 4 and b2 >> 6 shifts) coerces undefined to 0 via JS's own bitwise-operator ToInt32 conversion -- the same result the explicit ": 0" fallback gave. No input changes the output, only Uint8Array's own out-of-range-is-undefined semantics.
…lookup Adds three fixtures readEmbeddedOoxmlPayload's own decode had no direct coverage for: a nested archive's own same-named entry (ancestors.length > 0) must never overwrite the payload's genuine root-level part; the compound-file 'Package' stream must be found by its own name among several streams, not merely the first the directory tree visits (directory siblings are name-sorted, so a "Decoy" stream genuinely visits first); and bytes carrying neither the ZIP nor the compound-file magic must degrade to undefined. Also drops the function's own separate "is this even a ZIP or a compound file" gate: bytes matching neither magic still reach zipBytesOfPayload, fail isZipArchive, and then fail readCompoundFile's own equivalent magic check with a thrown CompoundFileFormatError -- caught by the same catch every other undecodable payload already degrades through. The gate changed which line produced undefined, never whether the caller saw it.
The previous commit removed readEmbeddedOoxmlPayload's own separate "neither ZIP nor compound file" early return, but this test's own name and comment still described that gate as the mechanism producing the undefined result. Both now describe how the catch block actually degrades this input.
Never published, but real code Stryker mutates all the same, and it had no test file of its own -- every existing use only exercised the default small/mini-stream shape indirectly through embedded.test.ts. Reads every fixture back through archive-codec's own independent readCompoundFile/readOlePackage, covering the custom stream-name option, a mini-stream payload spanning several sectors, and two differently-sized non-mini-stream (>= 4096 byte) payloads -- the large-stream code path no existing fixture ever reached.
…, style clamp Adds: a slide relationship filtered by its own type suffix rather than being the first one listed; an idx that names no shape falling back to type matching instead of returning early; a key naming neither idx nor type correctly refusing to match an equally-untyped shape; readRunPropertiesFromElement's own sizePt/bold/italic absence and explicit-false cases (no prior fixture omitted sz, or set b/i to anything but "1"); otherStyle as the fallback for a placeholder type that is neither title nor body; and level clamping at both the low (negative) and high (above 8) end, which the fixture's own single defined level (lvl1pPr) could only prove correct in one direction at a time.
The sizePt/bold-absence test's own rPr carried no i attribute either, but only sizePt and bold were checked -- italic's own undefined branch went unasserted, leaving it indistinguishable from an outer guard forced to always take the "===\"1\"" arm.
…tag multi-element residue
The existing two-cfRule-element fixture's own first element carried no
attributes at all, so a bypassed node-count check would still return
{} by coincidence. This one gives the first element real attributes,
so only the count check itself can tell a genuine single-element
residue apart from a multi-element one whose first entry happens to
match.
…forEach The name-encoding loop (writeEntry) and the magic-byte loop both copy a known array's own elements into a buffer with no bounds arithmetic of their own to get wrong -- forEach's own iteration removes the hand-written index comparison as a mutation target entirely, the same technique this package already uses elsewhere for a manually-bounded copy loop. Also drops writeEntry's own high-32-bits-of-size write: entry is always a fresh 128-byte slice of a zero-initialised directory buffer, so that byte is already 0 there, and every size this builder ever writes fits in 32 bits regardless.
…structure Adds five fixtures the round-trip-through-a-reader tests above cannot reach on their own: an exact-4096-byte packaged stream (the strict less-than boundary for "small"), direct inspection of every fixed [MS-CFB] header field this builder writes (several of which archive-codec's own reader deliberately never cross-checks -- its own header comments say so, for the directory's count fields and for the root entry's name specifically), the mini-FAT's own unused padding slot immediately past the real chain, and a fixture sized to the exact one-FAT-sector boundary this builder is structurally scoped to. The last two are not just belt-and-braces: manually verified against this exact suite, an off-by-one mutant on the mini-FAT loop's own bound writes into the byte-level fixture's padding slot with no other test able to observe it, and the boundary fixture is sized so a bypassed small-file guard elsewhere in this file provably raises RangeError against it while the real, guarded code still round-trips clean.
…eader writes 0x28, 0x48, and DIFAT[0] at 0x4c all write a literal 0 into file, a fresh Uint8Array that is already zero everywhere -- indistinguishable from leaving the default alone. Also rewrites the DIFAT[1..108] padding loop as an Array.from/forEach: its own last iteration is unobservable regardless of where the range ends, since the FAT sector's own bytes get (re)written immediately afterwards either way, so a hand-bounded comparison there was never provably correct by any test, only by inspection.
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.
Part of the workspace-wide effort to bring every package's Stryker mutation score to a genuine 100% with zero disable comments (see the sibling PRs already merged for byte-codec, excel-number-format, document-compute.js, pdf-raster-cpu).
Measured baseline for ooxml.js: 65.05% of 7126 valid mutants (killed 4604, timeout 31, survived 2081, no-coverage 410).
This PR is a work in progress. It has landed, in order:
test-support/cfb.ts), including byte-level assertions for header fields archive-codec's own reader deliberately never cross-checks, and a fixture sized to the builder's own one-FAT-sector boundary.Current measured score: 68.17% of 7043 valid mutants (killed 4771, timeout 30, survived 1882, no-coverage 360). Zero Stryker disable comments anywhere in the package.
Every fix is either a real test proving a genuine behavioural difference, or a small refactor removing code whose mutation was verified equivalent (a redundant guard made unreachable by the surrounding logic's own structure, a bitwise-shift's own out-of-range-is-undefined coercion, a zero write into an already-zero-initialised buffer, a hand-bounded copy loop replaced with
forEach/Array.fromover a known range) -- each verified directly by hand-mutating the source and confirming the existing suite still passed before the fix, and failed after it.Substantial work remains: the package's largest modules (
typed/docx/write.ts,typed/xlsx/build.ts,typed/docx/read.ts,typed/xlsx/conditional-format.ts,typed/xlsx/drawings-write.ts, and several others in the few-dozen-to-hundred-mutant range) are essentially untouched by this PR and still hold the bulk of the package's remaining survived/no-coverage mutants.stryker.config.ts'sbreakThresholdis left at its original measured-baseline value (63) rather than raised, since the package is not close to the genuine 100% that value would need to reflect.Left as a draft while this continues.