From 43d2646df8fbbee94427660cd66e5abc472d8260 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sat, 1 Aug 2026 22:25:29 +0200 Subject: [PATCH] test: resolve the first Stryker survivor queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triaging the survivors of the two smallest scoped mutation runs produced four distinct outcomes, three of which are addressed here. The fourth — twelve survivors covering the target-provided branch of the typed validation OpenAPI helpers — is deferred to #57, because that branch passes a null message and testing it now would encode the bug as the expected contract. Two survivors turned out to be false. Applying them by hand fails 46 and 52 of 112 tests respectively, yet Stryker reports both as survived: they sit in code reachable only during static initialization, which a reused test host runs once per process. Document that as a blind spot, since the report alone cannot settle such a survivor. Three survivors are unkillable and now carry narrow suppressions with their justification on the directive line, where Stryker records it — a reason split across lines is silently truncated in the report. Three were genuine guard-clause gaps: no test invoked the typed helpers on a null builder or ExecuteResultAsync on a null context, where the guards are what turn a NullReferenceException into the documented ArgumentNullException. Closes #66 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0184eh1G3XxvjRB11cNKyMRT --- ...-0-resolve-first-stryker-survivor-queue.md | 44 +++++++++++++++++++ ...tionErrorContractRegistrationExtensions.cs | 6 +++ ...ionOpenApiRouteHandlerBuilderExtensions.cs | 3 ++ tests/AGENTS.md | 25 ++++++----- .../LightActionResultGuardTests.cs | 19 ++++++++ .../ValidationOpenApiExtensionGuardTests.cs | 17 +++++++ 6 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 ai-plans/0066-0-resolve-first-stryker-survivor-queue.md create mode 100644 tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultGuardTests.cs diff --git a/ai-plans/0066-0-resolve-first-stryker-survivor-queue.md b/ai-plans/0066-0-resolve-first-stryker-survivor-queue.md new file mode 100644 index 0000000..cfea21b --- /dev/null +++ b/ai-plans/0066-0-resolve-first-stryker-survivor-queue.md @@ -0,0 +1,44 @@ +# Resolve the First Stryker Survivor Queue + +## Rationale + +The first mutation-testing triage pass covered the two smallest scoped runs, `Light.PortableResults.Validation.OpenApi` and `Light.PortableResults.AspNetCore.Mvc`, and produced twenty survivors. Twelve of them are one gap — the `target`-provided branch of the typed validation OpenAPI helpers is never exercised — and that branch calls `WithErrorExample` with a `null` message, which is the symptom described in issue #57. Testing it now would encode the current behavior as the expected contract, so it is deferred and resolved together with that bug. + +The remaining eight are independent and are the subject of this plan. Two are false survivors that expose a blind spot the tooling documentation does not yet describe, three are not killable and need justified suppression, and three are genuine guard-clause gaps of exactly the kind `tests/AGENTS.md` already asks for solitary tests. Resolving them leaves a survivor queue whose every remaining entry has a recorded reason, which is the state that makes future runs comparable. + +## Acceptance Criteria + +- [x] `tests/AGENTS.md` documents mutants in static initializers and static constructors as a blind spot, with the measured evidence and the reason the reused test host cannot kill them, so a future reader does not re-triage the same two mutants. +- [x] The two unreachable switch arms in `RegisterBuiltInValidationErrors` and the redundant `builder` guard in `ProducesPortableValidationProblemFor` carry narrow `Stryker disable once` comments with justifications; no global `ignore-mutations` setting is used. +- [x] Tests cover the null-builder guards of both typed validation OpenAPI helper families and the null-context guard of `BaseLightActionResult.ExecuteResultAsync`, asserting `ArgumentNullException`. +- [x] A re-run of both projects reports no survivor other than the twelve deferred to #57 and the two known-false static-initializer mutants; `AspNetCore.Mvc` reports zero survivors. +- [x] The baseline table in `tests/AGENTS.md` is replaced for both projects with full count vectors measured at the resulting commit, keeping the provenance the existing table records. +- [x] `dotnet test Light.PortableResults.slnx` passes, and production code changes are limited to suppression comments — no behavior changes, no CI changes, no mutation score thresholds. + +## Technical Details + +### Suppressions + +Three mutants cannot be killed by any test that respects the contract, so they are suppressed at the source with the reason recorded in the report: + +- `BuiltInValidationErrorContractRegistrationExtensions.cs`, the `ErrorMetadataTypeContract` arm — `BuiltInValidationErrorContracts.Contracts` is this method's only data source and contains schema and no-metadata contracts exclusively. The arm stays: it is correct for a registry that later gains a type contract, and removing it to satisfy the tool would be the restructuring `tests/AGENTS.md` forbids. +- The same method's `default:` arm message — `ErrorMetadataContract` declares a `private protected` constructor and has exactly three sealed subclasses, so no fourth kind can exist and the arm is unreachable by construction. +- `PortableValidationOpenApiRouteHandlerBuilderExtensions.cs`, the `ArgumentNullException.ThrowIfNull(builder)` guard — the delegated `ProducesPortableValidationProblem` guards the same parameter, so the observable contract is identical with or without it. The existing `ProducesPortableValidationProblemFor_ShouldRejectNullBuilder` test passes either way, which is why it did not kill the mutant. + +Match the mutator to the suppression (`Statement` for the two statement mutants, `String` for the message) rather than disabling all mutators at the site. + +### Guard clause tests + +The `EnsureBuilder` overloads in `BuiltInValidationErrorBuilderExtensions` are reached by all thirty-six public typed helpers, so one test per builder family is sufficient: one helper on `PortableProblemOpenApiBuilder` and one on `PortableValidationProblemOpenApiBuilder`, each invoked on a null builder. Without the guard these produce `NullReferenceException` instead of the documented `ArgumentNullException`, which is what makes the mutants killable. `ValidationOpenApiExtensionGuardTests` is the established home for these. + +`BaseLightActionResult.ExecuteResultAsync` needs the equivalent for a null `ActionContext`, reached through the sealed `LightActionResult`. The MVC test project currently has no unit-level test class; the integration tests exercise the app end to end and cannot reach this guard. + +### Blind spot: static initializers + +Applying the two false survivors to the source by hand fails the suite — emptying `BuiltInValidationErrorContracts.Contracts` fails 46 of 112 tests in `Light.PortableResults.Validation.OpenApi.Tests`, and blanking the built-in schema id string fails 52 of 112 — while Stryker reports both as survived. Both mutants sit in code reachable only during static initialization of a static property, which a reused test host runs once per process, before or independently of mutant activation. + +Document this next to the existing Safe Mode entry, in the same shape: what the tool reports, why, and what a reader must do instead. The consequence for triage is that a survivor in a static initializer, static constructor, or a helper called only from one is verified by applying it by hand and running the affected test project, not by reading the report. + +### Re-measurement + +Both projects are re-run at the end to produce the recorded baseline; together they take roughly four minutes. The suppressed mutants move from `Survived` to `Ignored`, which changes the `Ignored` count that `tests/AGENTS.md` currently explains as block-removal filtering alone — that explanation needs to account for user suppressions once they exist. diff --git a/src/Light.PortableResults.Validation.OpenApi/BuiltInValidationErrorContractRegistrationExtensions.cs b/src/Light.PortableResults.Validation.OpenApi/BuiltInValidationErrorContractRegistrationExtensions.cs index ea28d2f..2cc94c0 100644 --- a/src/Light.PortableResults.Validation.OpenApi/BuiltInValidationErrorContractRegistrationExtensions.cs +++ b/src/Light.PortableResults.Validation.OpenApi/BuiltInValidationErrorContractRegistrationExtensions.cs @@ -24,6 +24,9 @@ this ErrorMetadataContractsBuilder builder switch (contract) { case ErrorMetadataTypeContract typeContract: + // The built-in registry holds schema and no-metadata contracts exclusively, so this arm is + // unreachable today. It stays correct for a registry that later gains a type contract. + // Stryker disable once Statement : unreachable - the built-in registry never holds a type contract builder.ForCode(code, typeContract.MetadataType); break; case ErrorMetadataSchemaContract schemaContract: @@ -33,6 +36,9 @@ this ErrorMetadataContractsBuilder builder builder.ForCode(code); break; default: + // ErrorMetadataContract declares a private protected constructor and has exactly three sealed + // subclasses, so no fourth contract kind can exist and this arm cannot be reached. + // Stryker disable once String : unreachable - ErrorMetadataContract permits no fourth subclass throw new InvalidOperationException( $"The error metadata contract '{contract.GetType().FullName}' is not supported." ); diff --git a/src/Light.PortableResults.Validation.OpenApi/PortableValidationOpenApiRouteHandlerBuilderExtensions.cs b/src/Light.PortableResults.Validation.OpenApi/PortableValidationOpenApiRouteHandlerBuilderExtensions.cs index 8ef5d10..1c8b8e1 100644 --- a/src/Light.PortableResults.Validation.OpenApi/PortableValidationOpenApiRouteHandlerBuilderExtensions.cs +++ b/src/Light.PortableResults.Validation.OpenApi/PortableValidationOpenApiRouteHandlerBuilderExtensions.cs @@ -23,6 +23,9 @@ public static RouteHandlerBuilder ProducesPortableValidationProblemFor/reports/` (gitignored): JSON for agents (filter `"status"` for both `"Survived"` and `"Timeout"`; investigate the timeout cause before survivor triage), HTML for humans. +`-p` selects the mutated project, not the tests: Stryker runs every test project transitively referencing it (`AspNetCore.Shared` → 337 tests, `Light.PortableResults` → all 2,401). Cross-project kills are legitimate (sociable tests) and nearly free. Never pass `-tp` (it does not narrow tests) or `--since` (any non-C# file in the diff, e.g. an `ai-plans/` document, degrades it to a full run). Reports go to `StrykerOutput//reports/` (gitignored): JSON for agents (filter `"status"` for both `"Survived"` and `"Timeout"`; investigate the timeout cause before survivor triage), HTML for humans. ### Cost and baseline @@ -60,18 +60,20 @@ The smoke-check vector is tied to the current `Result.cs` and its tests; update | `AspNetCore.Mvc` | 33 | 11 | | `AspNetCore.MinimalApis` | 32 | 11 | -Baseline for sources at commit `04aee20`, remeasured with `dotnet-stryker` 4.16.0, concurrency 8 and 30,000 ms additional timeout (both pinned in `stryker-config.json`), `Debug`, Apple M3 Max (16 logical cores): +Baselines carry per-row provenance, because rows are re-measured individually as survivors are triaged. A row measured as part of the change that produced it cites the issue rather than a hash the commit cannot contain; find it with `git log --grep "Closes #"`. All runs used `dotnet-stryker` 4.16.0, concurrency 8 and 30,000 ms additional timeout (both pinned in `stryker-config.json`), `Debug`, Apple M3 Max (16 logical cores): -| Project | Tests run | Elapsed | Killed | Timeout | Survived | CompileError | Ignored | NoCoverage | -| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | -| `AspNetCore.Mvc` | 102 | 0:53 | 16 | 0 | 1 | 11 | 5 | 0 | -| `AspNetCore.MinimalApis` | 237 | 1:22 | 16 | 0 | 0 | 11 | 5 | 0 | -| `AspNetCore.Shared` | 337 | 2:29 | 31 | 0 | 0 | 3 | 10 | 0 | -| `Validation.OpenApi` | 163 | 2:38 | 57 | 0 | 19 | 2 | 36 | 0 | +| Project | Provenance | Tests run | Elapsed | Killed | Timeout | Survived | CompileError | Ignored | NoCoverage | +| --- | --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: | +| `AspNetCore.Mvc` | `#66` | 103 | 0:43 | 17 | 0 | 0 | 11 | 5 | 0 | +| `AspNetCore.MinimalApis` | `04aee20` | 237 | 1:22 | 16 | 0 | 0 | 11 | 5 | 0 | +| `AspNetCore.Shared` | `04aee20` | 337 | 2:29 | 31 | 0 | 0 | 3 | 10 | 0 | +| `Validation.OpenApi` | `#66` | 165 | 2:20 | 59 | 0 | 14 | 2 | 39 | 0 | -`Validation.OpenApi` was run twice consecutively with the 30,000 ms setting; both runs produced 57 killed, 0 timeout, 19 survived, and a 75.00% score, completing in 2:32 and 2:38. At 20,000 ms, one of two runs still timed out a mutant known to survive. At the 5,000 ms default, 21 mutants were reported as timeouts, masking most survivors and inflating the score to 98.68%. +Both `Validation.OpenApi` survivor groups are accounted for and neither is a missing test: twelve are the `target`-provided branch of the typed helpers, deferred to the bug in #57 so that tests are written against the corrected contract, and two are the known-false static-initializer mutants described in the blind spots below. Its 39 `Ignored` are 36 block-removal plus the three triage suppressions. -`Ignored` is not user suppression: all 56 baseline entries are `Block removal` mutants discarded deterministically by Stryker's built-in "block already covered" filter because another active mutant exists inside the block. A different reason or count should be investigated. +At the 5,000 ms default additional timeout, `Validation.OpenApi` reported 21 mutants as timeouts, masking most of these survivors and inflating the score to 98.68%. At 20,000 ms, one of two runs still timed out a mutant known to survive. + +`Ignored` covers two distinct things, and the report carries the reason for each. Most entries are `Block removal` mutants discarded deterministically by Stryker's built-in "block already covered" filter because another active mutant exists inside the block. The rest are `Stryker disable once` suppressions from triage, which carry the justification written at the source. Check the reasons, not just the count: a `Block removal` count that moves without a source change should be investigated. `NoCoverage` must be zero — a non-zero value means `coverage-analysis: off` is no longer taking effect. Compare full count vectors at equal concurrency, not percentages. @@ -96,5 +98,6 @@ If a survivor can only be killed by asserting on something incidental — exact Treat that list as observed, not fixed: any new `out`/`ref` code joins it silently. Stryker announces it as `[INF] Safe Mode! Stryker will remove all mutations in ` on the console only — no log file is written — and the discarded mutants are simply absent from the JSON report. The durable way to recover the current set is to filter the report for `"status": "CompileError"`; those sites are the only trace left, and their enclosing methods are the ones running blind. When changing a method in that set, mutation score carries no information about it and line coverage only proves execution. Adequacy has to be argued by hand: enumerate the behaviors the method promises and point at the test constraining each one. State that reasoning in the pull request, because no tool in this repository can check it. +- Mutants reachable only during static initialization are reported as survived even when the suite kills them. Measured on `BuiltInValidationErrorContracts`: emptying the `Contracts` registry fails 46 of 112 tests and blanking the built-in schema id string fails 52 of 112, yet Stryker reported both as `Survived`. Both sites run only while a static property initializer executes, which a reused test host runs once per process, independently of mutant activation. Verify any survivor in a static initializer, a static constructor, or a helper called only from one by applying the mutation to the source by hand and running the affected test project — the report cannot settle it. Do not add tests for such a survivor before that check: the two above already had covering assertions. - `Timeout` counts as killed. The pinned 30,000 ms additional timeout reduced `Validation.OpenApi` from 21 timeouts to zero in two consecutive concurrency-8 runs, but no finite value makes classification independent of hardware and load. Investigate any future timeout as either a genuine hang or insufficient headroom; do not assume it represents a killed mutant. - The MTP runner is a preview (stryker-mutator/stryker-net#3094); verify surprising results against a plain `dotnet test` run. diff --git a/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultGuardTests.cs b/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultGuardTests.cs new file mode 100644 index 0000000..cf8db41 --- /dev/null +++ b/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultGuardTests.cs @@ -0,0 +1,19 @@ +using System; +using System.Threading.Tasks; +using FluentAssertions; +using Xunit; + +namespace Light.PortableResults.AspNetCore.Mvc.Tests; + +public sealed class LightActionResultGuardTests +{ + [Fact] + public async Task ExecuteResultAsync_ShouldRejectNullContext() + { + var actionResult = new LightActionResult(Result.Ok()); + + var act = async () => await actionResult.ExecuteResultAsync(null!); + + await act.Should().ThrowAsync(); + } +} diff --git a/tests/Light.PortableResults.Validation.OpenApi.Tests/ValidationOpenApiExtensionGuardTests.cs b/tests/Light.PortableResults.Validation.OpenApi.Tests/ValidationOpenApiExtensionGuardTests.cs index 49a2a52..35e248c 100644 --- a/tests/Light.PortableResults.Validation.OpenApi.Tests/ValidationOpenApiExtensionGuardTests.cs +++ b/tests/Light.PortableResults.Validation.OpenApi.Tests/ValidationOpenApiExtensionGuardTests.cs @@ -1,5 +1,6 @@ using System; using FluentAssertions; +using Light.PortableResults.AspNetCore.OpenApi; using Light.PortableResults.AspNetCore.OpenApi.ErrorContracts; using Microsoft.AspNetCore.Builder; using Xunit; @@ -24,4 +25,20 @@ public void ProducesPortableValidationProblemFor_ShouldRejectNullBuilder() act.Should().Throw(); } + + [Fact] + public void TypedProblemHelpers_ShouldRejectNullBuilder() + { + var act = static () => ((PortableProblemOpenApiBuilder) null!).WithEqualToError(); + + act.Should().Throw(); + } + + [Fact] + public void TypedValidationProblemHelpers_ShouldRejectNullBuilder() + { + var act = static () => ((PortableValidationProblemOpenApiBuilder) null!).WithEqualToError(); + + act.Should().Throw(); + } }