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(); + } }