Assert what the mutation gate found unasserted in the two test-support packages - #324
Merged
Merged
Conversation
The mutation gate scored this package at 45 %: its happy paths were covered from FirstClassErrors.UnitTests, but four families of behaviour were executed without being asserted, and could be deleted with the suite staying green. Argument guards. Every public entry point documents an ArgumentNullException it never proved: removing any of those throws changed nothing. Note that Clock.Use and InstanceIds.Use are not the same case — Clock forwards a lambda and never the argument, so its guard is the only one there is. Failure messages. This package's product is what it prints when an expectation fails, and every one of those strings could be replaced by an empty one unnoticed. Each message is now required to name both the expected and the actual value, an absent context entry to list the keys that are present, and a null to render as null rather than as an empty string. Arbitrary-value shapes. `Any detailed message 7F3A9C.` announces itself as arbitrary where a bare random string would read as a value someone chose. Nothing asserted the prefix, the trailing period, or that the factories vary at all — a factory pinned to a constant would have passed. The context-key separator check reads the key list alone rather than the whole message: the surrounding prose carries commas of its own, and a check over the full string passes whether or not the separator survived.
Two argument guards cannot be killed by any test, and the reason is worth stating next to the code rather than rediscovering at the next sweep. WithContextEntry's guard duplicates one the dictionary lookup below already performs, with the same exception and the same parameter name. InstanceIds.Use duplicates the guard inside AmbientInstanceId.Use, identically. Removing either is unobservable; both stay because they state the contract where the caller reads it. Clock.Use is deliberately not marked: it forwards a lambda and never the argument, so its guard is the only one there is — and a test now proves it. Each carries a `// Stryker disable once Statement,Block` with its reason. Both mutators are named because Stryker generates both on such a guard and marking only one leaves the other surviving for the very reason the comment gives; the `Equality` mutator is deliberately left out, since a test kills it and it must stay counted. A lowered threshold would have hidden every future survivor along with these two.
The suite observes the attribute from inside a decorated test, which is the right way to prove that pinning works but leaves the hooks' own behaviour unseen: a test never watches its own After run, so nothing established that the scope was closed rather than abandoned, nor what happens when After runs without a matching Before. Four behaviours are now asserted: a declared seed is read back, an undeclared one reads as zero, After without a scope is a no-op rather than a NullReferenceException, and After releases the ambient source instead of leaking the seed into whatever runs next. A fifth covers the nesting the method, class and assembly levels rely on — closing the inner scope must restore the outer seed, not the unpinned source. Both hooks ignore their two parameters, so they are passed as null: naming a real MethodInfo and a fake IXunitTest would suggest they matter. Two survivors are deliberately left. Writing the report to the test output and reading the finished test's outcome are only observable from a test that has already failed, which is exactly why ReportFor was extracted — the rule is tested, the wiring is not.
Both bars were calibrated on scores that no longer hold. `FirstClassErrors.Testing` was set at 40 when it measured 45 %; it now measures 97 %. `JustDummies.Xunit` was set at 50 when it measured 57 %; it now measures 86 %. Left where they were, the bars would have permitted the regression of everything just asserted without a single check turning red — which is the one thing a ratchet exists to prevent. They move to 90 and 80: below the measured scores, with room for one equivalent mutant to appear under a later refactoring, and far enough above the previous values to hold the ground that was gained. Four survivors remain and none is a missing test. Two are the unreachable null branch of OutcomeAssertions.Describe — Outcome<T>.Success rejects null, so a successful outcome cannot carry one — left uncounted rather than marked, because a marker would have silenced the two killed mutants sharing that line. The other two are the Reproducible attribute writing its report and reading the finished test's outcome, both observable only from a test that has already failed.
…tion-testing-4okfe0
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
2 participants
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
The mutation gate scored
FirstClassErrors.Testingat 45 % andJustDummies.Xunitat 57 %. This pull request asserts the behaviours those scores pointed at, marks the mutants that genuinely cannot be killed, and ratchets both thresholds up to the ground gained.breakFirstClassErrors.TestingJustDummies.XunitType of change
Changes
The low scores were not "the API is untested" — the happy paths were already covered from
FirstClassErrors.UnitTests. What survived was four families of behaviour that tests executed without asserting:ArgumentNullExceptionthat nothing proved; each of thosethrows could be deleted with the suite staying green.nullto render asnullrather than as an empty string.Any detailed message 7F3A9C.announces itself as arbitrary where a bare random string would read as a value someone chose. Nothing asserted the prefix, the trailing period, or that the factories vary at all.Afterrun, so nothing established that the scope was closed rather than abandoned, nor what happens whenAfterruns without a matchingBefore.The survivors that remain, and why
Four, none of them a missing test — each verified by reading the code it depends on rather than assumed:
OutcomeAssertions.Describe, null branch (2). Unreachable:Outcome.Successrejects null, so a successful outcome cannot carry one. Left uncounted rather than marked, because a marker would have silenced the two killed mutants sharing that line.ReproducibleAttribute, writing the report and reading the outcome (2). Observable only from a test that has already failed. This is the limit the design anticipated when it extractedReportFor: the rule is tested, the wiring is not. Closing it would mean injecting an output sink into the attribute — a design change, not a test, and not one to make unasked.Two further guards were equivalent and are now marked in place with
// Stryker disable once Statement,Blockand their reason:WithContextEntry's guard duplicates one the dictionary lookup performs with the same exception and parameter name, andInstanceIds.Useduplicates the guard insideAmbientInstanceId.Use. Both mutators are named because Stryker generates both on such a guard;Equalityis deliberately left out, since a test kills it and it must stay counted.Clock.Uselooks like the same case and is not — it forwards a lambda and never the argument, so its guard is the only one there is. It carries no marker, and a test now proves it.Testing
dotnet build FirstClassErrors.slndotnet test FirstClassErrors.slnFirstClassErrors.Analyzers.UnitTests)Whole solution green: 1 844 tests, 0 failures, across the 13 test projects.
Both thresholds were verified without
--break-at 0, so the process exit code proves the bar actually holds:testingexits 0 at 97.26 % against 90,justdummies-xunitexits 0 at 85.71 % against 80. Every score quoted here comes from a serialised run with noPendingmutants — two earlier runs were disturbed by concurrent Stryker processes and their numbers discarded.Re-measured after merging
mainmainmoved to6759b94while this branch was open, and #323 rewrote the ambient seed scopes inRandomSourceinto a linked stack tolerating out-of-order disposal — the exact machineryReproducibleAttributeHookTestsasserts throughNestedScopesUnwindInOrderandTheAfterHookClosesTheScope. The merge is textual-conflict-free and touches none of this branch's files, but that is not on its own an argument that the behaviour still holds, so both were re-run against the merged tree: the whole solution stays green, and both scores come back identical —testing71 killed / 2 survived (97.26 %),justdummies-xunit12 / 2 (85.71 %), noPendingmutants, both exiting 0 against the raised thresholds.Documentation
doc/updateddoc/handwritten/for-users/README.fr.md) updated if user-facing behavior changedNo user-facing behaviour changed. The two source edits are comments; the reasoning they carry lives next to the code it explains, which is where the next sweep will need it.
Architecture decisions
Proposed: ADR-____This applies ADR-0043's ratchet rather than deciding anything new.
Related issues
None.
Generated by Claude Code