test(byte-codec): eliminate every mutant by restructuring rather than suppressing - #1252
Conversation
…h no EOI readJpegInfo's marker-scanning loop must stop once it runs off the end of the buffer even when the trailing bytes contain neither a marker lead-in (0xff) nor an EOI marker. Adds a case exercising exactly that: trailing non-marker bytes with nothing after them, expecting the "no SOF marker found" error rather than an out-of-bounds read.
…unk guard Pushing an empty chunk is a no-op either way: toBytes() and length are identical whether or not the guard runs, since an empty chunk contributes zero bytes to both the running length and the concatenated output. The early return existed only as an allocation avoidance, not a behavioural branch, so there is no mutation opportunity left for it to hide.
…he data itself Two changes to inflateTolerant's recovery ladder, each removing a comparison that no input could ever distinguish: - The whitespace-skip loop no longer pairs its scan with a separate offset < data.length bound. isAsciiWhitespace(undefined) is explicitly false and Uint8Array indexing past the end always returns undefined, so the scan already stops the moment it runs off the buffer without needing its own length check. - The offset > 0 guard before retrying inflate() on the whitespace-stripped subarray is gone. inflate() is a deterministic pure function, so retrying it at offset 0 (the identical bytes the first attempt already threw on) fails the same way and falls through to the next recovery tier regardless.
readJpegInfo's marker-scanning loop no longer pairs its scan with a separate offset < bytes.length bound. Uint8Array indexing past the end always returns undefined, so while (bytes[offset] !== undefined) already stops the scan the moment it runs off the buffer, with no separate length comparison for a boundary mutation to hide behind.
… PNG decoding Four changes to png-decode.ts, each removing a comparison or branch that no input could ever observe: - unpackRow's dedicated bitDepth === 8 fast path is gone. With bitDepth === 8, the generic bit-packed formula already reduces to exactly the fast path's own computation (mask = 255, byteIndex = i, shift = 0), so the branch existed purely to skip redundant shift/mask arithmetic, never to produce a different result. - unpackRow's two remaining sample loops (bitDepth 16 and the generic bit-packed case) are built via an exact-length Array.from instead of a manually bounded for loop, so there is no separate loop-bound comparison whose own off-by-one could ever be observed through the returned array. - decodePng's palette lookup runs unconditionally instead of being gated on colorType === 3. buildRawImage only ever reads it inside its own colorType === 3 branch, so finding a PLTE chunk for any other colour type is simply an unused value. - buildRawImage's row and column loops are likewise driven by an exact-length Array.from: data/alpha are allocated to exactly width*height*outChannels/width*height elements, so there is no separate loop-bound comparison for an off-by-one to hide behind.
… without pairwise comparisons Four changes to png-filter.ts, each removing a comparison that no input could ever distinguish: - paethPredictor now picks whichever of a, b, c has the smallest distance directly via Math.min, rather than a chain of pairwise comparisons (pa <= pb, then pb <= pc). The chain's own tie boundary (pa <= pb vs pa < pb) was unobservable: pa === pb algebraically forces pc === 0, which the second comparison already resolves independently, so no input could tell the two apart. - sumOfAbsSigned computes each byte's signed-interpretation magnitude via Math.min(byte, 256 - byte) instead of a byte < 128 branch. Both formulations agree everywhere, including at the branch's own boundary (byte === 128, where both magnitudes are already 128), so there is no comparison left to mutate at all. - unfilterScanlines' and filterRowInto's per-row byte loops are built via an exact-length Array.from instead of a manually bounded for loop, so there is no separate loop-bound comparison whose own off-by-one could ever be observed through their output arrays.
…d drop redundant loop bounds Two changes to png-encode.ts: - detectPalette's per-pixel Map key packs r/g/b/a into one 32-bit bitfield (r | g << 8 | b << 16 | a << 24) instead of summing scaled terms (r + g*256 + b*65536 + a*16777216). Each channel now occupies its own disjoint 8-bit lane, so the packing is a bijection by construction, with no arithmetic identity between coefficients for a mutation to preserve the way the scaled-sum form had. - writeTruecolorPng's pixel and channel interleaving loops are built via an exact-length Array.from instead of manually bounded for loops, so there is no separate loop-bound comparison whose own off-by-one could ever be observed through the interleaved output.
…able comments The break threshold's own comment still described the old suppression mechanism (per-mutant Stryker disable comments with an equivalence proof). None remain: every mutation opportunity that was genuinely unobservable has instead been restructured out of the source, so the comment now describes the restructuring patterns actually used instead of pointing at comments that no longer exist.
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. |
|
Local Stryker verification note: every I wasn't able to get a trustworthy local mutation score for this PR: the machine this was built on was running many concurrent sessions doing the identical "remove Stryker disable comments" pass across other packages in this workspace at the same time (visible in the Actions run history for fix/no-disable-comments-pdf-raster-cpu, fix/no-disable-comments-document-compute.js, fix/no-disable-comments-excel-number-format, feat/100-percent-mutation-wpd-codec), which pushed load averages into the 60-400 range on a 12-core box. Every full run either timed out near-universally or, once, reported widespread "survived" mutants that don't hold up: e.g. it reported flate.ts's The repo's own |
|
Update: got a genuine, clean local run after the workspace-wide Real result from 0 survived, 0 no-coverage, mutation score 100.00 (>= break threshold 100). The two non-"Killed" categories are both explainable, not noise:
Verified against the report's raw |
|
🎉 This PR is included in version 1.5.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
byte-codec previously reached a 100% Stryker mutation score using
// Stryker disable next-line <Mutator>: <reason>suppression comments to exclude genuinely unobservable mutants. That mechanism is now banned workspace-wide: every mutant must be killed by a real test, or the code restructured so the mutation opportunity doesn't exist as an AST node at all.This removes every remaining disable comment from byte-codec (writer.ts, flate.ts, jpeg-info.ts, png-decode.ts, png-encode.ts, png-filter.ts) by restructuring each equivalence class rather than suppressing it:
ByteWriter.writeBytes's empty-chunk early return,inflateTolerant'soffset > 0retry guard).while (bytes[offset] !== undefined)) or exact-lengthArray.fromiteration, removing the separate length comparison an off-by-one mutant could hide behind.paethPredictor's tie-breaking viaMath.min,sumOfAbsSigned's signed-magnitude fold viaMath.min(byte, 256 - byte).r | g << 8 | b << 16 | a << 24) instead of a sum of scaled terms, removing the arithmetic-identity mutation surface entirely.Mirrors the same approach already landed for
excel-number-formatin this workspace.Typecheck, lint, and the full unit suite (208 tests) are green.
grep -rn "Stryker disable" packages/byte-codec/srcreturns nothing.