fix(mtp): analyse projects without a design-time build - #3672
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a failure mode when running Stryker with --test-runner mtp against strong-named projects by ensuring the emitted mutant assembly keeps the same AssemblyVersion as the originally built assembly, preserving assembly binding for the unchanged test assembly.
Changes:
- Read the original built assembly’s version (metadata-only) and inject a matching
[assembly: AssemblyVersion(...)]into the mutated compilation when needed. - Remove any existing assembly-level
AssemblyVersionattributes from syntax trees to avoid duplicate-attribute compilation errors before injecting the replacement. - Add unit tests that verify the mutant version is forced to match the original, and that behavior is a no-op when the original assembly version cannot be read.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Stryker.Core/Stryker.Core/Compiling/CsharpCompilingProcess.cs | Forces mutated compilation’s AssemblyVersion to match the original assembly to prevent strong-name binding failures under MTP. |
| src/Stryker.Core/Stryker.Core.UnitTest/Compiling/CSharpCompilingProcessTests.cs | Adds coverage ensuring version alignment occurs when possible and is skipped when the original cannot be read. |
| // recompiles from the source trees gathered via a design-time build where those tools don't run, | ||
| // so the version would otherwise default to 1.0.0.0. For a strong-named assembly that downgrade |
| /// <summary> | ||
| /// Ensures the mutated assembly is emitted with the same <see cref="AssemblyVersionAttribute"/> as the | ||
| /// original built assembly, so its identity keeps matching the references of the (unchanged) test assembly. | ||
| /// Without this, the version of the recompiled assembly defaults to 1.0.0.0 and strong-named projects can |
| // The mutated source has no AssemblyVersion attribute, so without intervention it would compile to | ||
| // the default 1.0.0.0. Stryker must instead emit the version of the original built assembly (here | ||
| // 7.3.2.1) so the unchanged test assembly can still bind to the mutant - critical for strong-named | ||
| // assemblies where the loader enforces the version. See https://github.com/stryker-mutator/stryker-net/issues/3094. |
|
Disclaimer, thanks for the proposal, but it looks like a bolt on patch for a very specific issue that affect the overall behavior and I feel it would be good to have an understanding on the issue at hand to identify the right solution. Assuming analysis is correct (*), it would be simpler, and faster, to use a regular build instead of a design time one. Also, I wonder why this problem surfaces just now, as I see not recent change that would explain it. So, we have, at least, 3 possibilities:
(*) I consider the analysis is an assumption since fixing this by retrieving version number from the generated assembly would work disregarding the reason of version generation failing. |
Stryker analysed projects with a Buildalyzer design-time build, which sets DesignTimeBuild=true. That makes versioning tools such as MinVer skip version generation, so the source Stryker captures (and recompiles the mutant from) carried the default 1.0.0.0 instead of the real assembly version. For a strong-named project the unchanged test assembly still references the original version, the test host cannot bind the downgraded mutant, and it crashes before any test runs (reported in stryker-mutator#3094, seen with Polly under the MTP runner). Run a regular build for analysis instead, so the version is computed exactly as in a normal build, while keeping SkipCompilerExecution=true (plus the output-copy suppressions) so csc never runs and analysis stays fast and side-effect free. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
627df89 to
38d3405
Compare
|
@dupdob I agree that this isn't the right approach, it's "addressing the symptom". From what you've said, I think the most appropriate approach is no not do a design-time build for the analysis and workspace initialization. This updated PR now does this, and also addresses the issue. I was keen to keep the MSBuild properties in a way that prevent Buildalyzer from emitting assemblies, but to not do a design-time build, as we are kind of creating a full build pipeline. Let me know what you think. |
|
I'm also of the opinion that Buildalyzer should be dropped in favour of a stryker out-of-process build host, which I've prototyped here: |
But why? what problem will it solve ? we have accumulated years of experience working with Buildalyzer, learning and dealing the the intricacies of various SDKs, why would we want to start over? |
|
Thinking about it, this clearly appears to be a nice issue: it happens when one uses
Or maybe a combination of both, that would bring out the best:
|
I've faced issues in the past that I've attributed to Buildalyzer (which I've not raised issues for, so not the best citizen here), and at work I process hundreds of projects daily using Buildalyzer, it had a success rate of about 80-something percent, with MSBuildWorkspace that's in the high 90s. Buildalyzer solved a problem when it was first introduced, however since mid 2024 the new rework in I'm not being pushy with this, just saying that there is an alternative implementation (behind an opt-in flag) that we could look at if we ever face odd bugs, rather than having to back-and-forth between Stryker and Buildalyzer. |
|
Closing as the approach here needs considering and discussing. |
|
I also agree that it's time we look at msbuild workspace again, it's come up multiple times in the past but it's not an easy choice to make. Even adding it as opt in creates a maintenance burden we need to consider. I am willing to make that consideration at this point, we will discuss it soon and decide the path forward. |
|
The main benefit of switching to msbuild workspace is to get rid of Buildalayzer dependency. Which would probably reduce reaction times as there are far more contributors to Stryker.Net than to Buildalyzer. An alternative would be for Buildalyzer to switch to said workspaces 😄 . |
Problem
Running Stryker with
--test-runner mtpagainst a strong-named project fails: every mutant printsE(runtime error), Stryker logs "Stryker was unable to calculate a mutation score", and then exits successfully — masking that nothing was actually tested. Reported by @martincostello while trying MTP on Polly (#3094 comment, App-vNext/Polly#3131).Root cause
E=MutantStatus.RuntimeError, which Stryker assigns when the MTP test host crashes during a run. The host dies with an unhandled exception on every run:Comparing assembly identities:
Polly.Core.dllThe assembly version is normally injected by MSBuild — or by tools such as MinVer / Nerdbank.GitVersioning (Polly uses MinVer) — during a real build. But Stryker analyses projects with a Buildalyzer design-time build, which sets
DesignTimeBuild=true. MinVer (among others) deliberately skips version generation on a design-time build, so the source Stryker captures — and recompiles the mutant from — carries the default1.0.0.0instead of the real version.For a strong-named assembly that downgrade is fatal: the unchanged test assembly still references
Polly.Core, Version=8.0.0.0, the loader cannot bind a lower-versioned assembly, the test host throws on a background thread and exits before any test runs. Non-strong-named assemblies ignore the version, which is why this only surfaces on signed projects.Fix
Stop using a design-time build for analysis. Stryker now runs a regular build (so MSBuild computes the version exactly as a normal build would), while keeping
SkipCompilerExecution=true— plus the output-copy / project-reference suppressions — so the C# compiler never runs and analysis stays fast and side-effect free.Concretely, the Buildalyzer
EnvironmentOptionsis configured withDesignTime = falseand a curated set of global properties that reproduce the cheap parts of a design-time build but omit the two flags that cause the problem:DesignTimeBuild(suppresses version generation) andBuildingProject(short-circuits theBuildtarget). This is general — it fixes any strong-named project whose version comes from MinVer / Nerdbank.GitVersioning / GitVersion / etc., with no per-repo configuration.Verification
Validated through Buildalyzer's own API (replicating Stryker's exact analysis call) against a strong-named MinVer project tagged
8.0.0:AssemblyVersion1.0.0.0❌8.0.0.0✅Identical captured source files and references, the correct version, and the compiler is still skipped.
That the corrected version is what unblocks Polly was confirmed independently: forcing the analysed version to match the real one (via
MinVerSkipon an otherwise-unfixed Stryker) takes the realPolly.Core(xunit v3 + MTP) from0 mutations covered/ allEto1324 mutations covered, real results (.....SS.S.S...S.S), final score 81.82%, zeroEs. This change achieves the same by making the analysis build produce the real version in the first place.Added a unit test asserting analysis uses a non-design-time build that still skips compilation. All Initialisation + Compiling unit tests pass.
Reported in #3094
🤖 Generated with Claude Code