feat(MTP): per-test coverage analysis for MTP runner#3516
feat(MTP): per-test coverage analysis for MTP runner#3516piotr-nawrot-golba-music wants to merge 7 commits intostryker-mutator:masterfrom
Conversation
…ureCoverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ution MTP runner now captures per-test coverage by running each test in an isolated process. When coverage-analysis is set to perTest or perTestInIsolation, each test gets its own MTP server process. The server is stopped after each test, triggering MutantControl.FlushCoverageToFile() via ProcessExit, and the resulting coverage file is read to build per-test coverage results. This enables Stryker to determine which tests cover which mutants for MTP-based frameworks (xUnit v3, TUnit, MSTest with MTP, NUnit with MTP), unlocking coverage-based test optimization that was previously only available with VsTest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements per-test coverage capture for the Microsoft Testing Platform (MTP) runner by executing each test in a dedicated server process lifecycle (start → run → stop → read coverage), enabling coverage-based test selection in MTP similar to existing runners.
Changes:
- Added single-test coverage execution path in
SingleMicrosoftTestPlatformRunner, including stopping/removing servers to trigger coverage flush. - Updated
MicrosoftTestPlatformRunnerPool.CaptureCoverage()routing to use per-test capture whenCoverageBasedTestis enabled, with confidenceNormalvsExactdepending onCaptureCoveragePerTest. - Added/updated unit tests and a testable runner override to support per-test coverage capture scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Stryker.TestRunner.MicrosoftTestPlatform/SingleMicrosoftTestPlatformRunner.cs | Adds server stop/removal and single-test coverage capture API. |
| src/Stryker.TestRunner.MicrosoftTestPlatform/MicrosoftTestPlatformRunnerPool.cs | Routes coverage capture to per-test vs aggregate and adds parallel per-test capture loop. |
| src/Stryker.TestRunner.MicrosoftTestPlatform.UnitTest/TestableRunner.cs | Adds override hook to simulate per-test coverage results in pool tests. |
| src/Stryker.TestRunner.MicrosoftTestPlatform.UnitTest/SingleMicrosoftTestPlatformRunnerCoverageTests.cs | Adds tests intended to cover new server stop/removal and per-test coverage behaviors. |
| src/Stryker.TestRunner.MicrosoftTestPlatform.UnitTest/MicrosoftTestPlatformRunnerPoolTests.cs | Adds unit tests validating per-test routing and confidence selection in the pool. |
| try | ||
| { | ||
| DeleteCoverageFile(); | ||
|
|
||
| var server = await GetOrCreateServerAsync(assembly).ConfigureAwait(false); | ||
| await server.RunTestsAsync(new[] { test }).ConfigureAwait(false); | ||
| await StopAndRemoveServerAsync(assembly).ConfigureAwait(false); | ||
|
|
||
| var (coveredMutants, staticMutants) = ReadCoverageData(); |
There was a problem hiding this comment.
RunSingleTestForCoverageAsync only stops/removes the server and deletes the coverage file on the success path. If GetOrCreateServerAsync, RunTestsAsync, or ReadCoverageData throws, the catch returns a result but leaves the server running/cached and may leave the coverage file behind, which can leak processes and/or contaminate subsequent per-test captures. Consider moving StopAndRemoveServerAsync(assembly) and coverage-file cleanup into a finally (best-effort) so they run even when an exception occurs.
| private IEnumerable<ICoverageRunResult> CaptureCoverageTestByTest( | ||
| IProjectAndTests project, CoverageConfidence confidence) | ||
| { | ||
| _logger.LogInformation("Starting per-test coverage capture for MTP runner"); | ||
|
|
||
| foreach (var runner in _availableRunners) | ||
| { | ||
| runner.SetCoverageMode(true); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var allTests = new List<(string Assembly, TestNode Test, string TestId)>(); | ||
| foreach (var (assembly, tests) in _testsByAssembly) |
There was a problem hiding this comment.
CaptureCoverageTestByTest accepts an IProjectAndTests project parameter but doesn’t use it. Right now it iterates _testsByAssembly (all discovered tests) regardless of the project’s current test assemblies, which could be incorrect if the pool is reused across projects/assembly sets. Either use project.GetTestAssemblies() to scope which assemblies/tests are captured, or remove the unused parameter to avoid misleading callers.
| [TestMethod] | ||
| public async Task RunSingleTestForCoverageAsync_ShouldReturnCoverageFromFile() | ||
| { | ||
| var runnerId = 620; | ||
| var coverageFilePath = Path.Combine(Path.GetTempPath(), $"stryker-coverage-{runnerId}.txt"); | ||
|
|
||
| try | ||
| { | ||
| using var runner = new SingleMicrosoftTestPlatformRunner( | ||
| runnerId, | ||
| _testsByAssembly, | ||
| _testDescriptions, | ||
| _testSet, | ||
| _discoveryLock, | ||
| NullLogger.Instance); | ||
|
|
||
| File.WriteAllText(coverageFilePath, "1,2,3;10"); | ||
|
|
||
| var result = runner.ReadCoverageData(); | ||
|
|
There was a problem hiding this comment.
This test is named RunSingleTestForCoverageAsync_ShouldReturnCoverageFromFile, but it never calls RunSingleTestForCoverageAsync; it writes a coverage file and asserts ReadCoverageData() output instead (which is already covered by existing ReadCoverageData_* tests above). This leaves the newly added RunSingleTestForCoverageAsync behavior untested (server lifecycle + confidence + Dubious fallback). Update the test to actually invoke RunSingleTestForCoverageAsync (or rename it to match what it validates).
| [TestMethod] | ||
| public async Task RunSingleTestForCoverageAsync_ShouldReturnCoverageFromFile() | ||
| { | ||
| var runnerId = 620; | ||
| var coverageFilePath = Path.Combine(Path.GetTempPath(), $"stryker-coverage-{runnerId}.txt"); | ||
|
|
||
| try | ||
| { | ||
| using var runner = new SingleMicrosoftTestPlatformRunner( | ||
| runnerId, | ||
| _testsByAssembly, | ||
| _testDescriptions, | ||
| _testSet, | ||
| _discoveryLock, | ||
| NullLogger.Instance); |
There was a problem hiding this comment.
RunSingleTestForCoverageAsync_ShouldReturnCoverageFromFile is declared as async Task but contains no await, which produces CS1998 and can hide test failures (exceptions may be observed differently). Either remove async/return Task and make it synchronous, or add awaits by actually invoking the async API under test.
| [TestMethod] | ||
| public void RunSingleTestForCoverageAsync_ShouldReturnDubious_WhenNoCoverageFile() | ||
| { | ||
| var runnerId = 621; | ||
| using var runner = new SingleMicrosoftTestPlatformRunner( | ||
| runnerId, | ||
| _testsByAssembly, | ||
| _testDescriptions, | ||
| _testSet, | ||
| _discoveryLock, | ||
| NullLogger.Instance); | ||
|
|
||
| var result = runner.ReadCoverageData(); | ||
|
|
||
| result.CoveredMutants.ShouldBeEmpty(); | ||
| result.StaticMutants.ShouldBeEmpty(); | ||
| } |
There was a problem hiding this comment.
RunSingleTestForCoverageAsync_ShouldReturnDubious_WhenNoCoverageFile also doesn’t call RunSingleTestForCoverageAsync and can’t assert the expected Dubious confidence because it only calls ReadCoverageData() (which returns empty lists on missing file). If the intent is to verify the new per-test method’s error path, invoke RunSingleTestForCoverageAsync and assert it returns CoverageConfidence.Dubious (and that it performs any required cleanup).
When RunSingleTestForCoverageAsync gets empty coverage data (e.g., server force-killed before FlushCoverageToFile ran), the result now correctly uses CoverageConfidence.Dubious instead of the requested confidence level. This prevents silently marking mutants as uncovered when coverage capture failed. Also fixes misleading test names (renamed RunSingleTestForCoverageAsync_* to ReadCoverageData_* where they only tested ReadCoverageData), and adds proper tests for the Dubious confidence paths through the pool. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for async-safe concurrency - Replace AutoResetEvent with SemaphoreSlim in RunnerPool.RunThisAsync to avoid blocking thread-pool threads during runner checkout - Replace object _serverLock with SemaphoreSlim(1,1) in SingleRunner to enable holding the lock across await in GetOrCreateServerAsync, eliminating the TOCTOU race in the check-create-start-store pattern - Fix CalculateAssemblyTimeout to snapshot _testDescriptions once instead of acquiring _discoveryLock per LINQ element Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| var server = await GetOrCreateServerAsync(assembly).ConfigureAwait(false); | ||
| await server.RunTestsAsync(new[] { test }).ConfigureAwait(false); | ||
| await StopAndRemoveServerAsync(assembly).ConfigureAwait(false); |
There was a problem hiding this comment.
RunSingleTestForCoverageAsync only stops/removes the assembly server on the success path. If RunTestsAsync (or server creation) throws, the catch returns Dubious without stopping the server or removing it from _assemblyServers, which can (a) leave a test server process running, and (b) cause later per-test runs to reuse the same process, defeating the intended “stop between tests to flush coverage” isolation. Consider moving StopAndRemoveServerAsync(assembly) into a finally (with a null/server-created guard as needed) so cleanup happens even on exceptions.
| var server = await GetOrCreateServerAsync(assembly).ConfigureAwait(false); | |
| await server.RunTestsAsync(new[] { test }).ConfigureAwait(false); | |
| await StopAndRemoveServerAsync(assembly).ConfigureAwait(false); | |
| try | |
| { | |
| var server = await GetOrCreateServerAsync(assembly).ConfigureAwait(false); | |
| await server.RunTestsAsync(new[] { test }).ConfigureAwait(false); | |
| } | |
| finally | |
| { | |
| await StopAndRemoveServerAsync(assembly).ConfigureAwait(false); | |
| } |
| if (!_availableRunners.TryTake(out var runner)) | ||
| { | ||
| // Another thread grabbed the runner between the semaphore release and our TryTake; re-wait | ||
| _runnerAvailable.Release(); | ||
| return await RunThisAsync(task).ConfigureAwait(false); |
There was a problem hiding this comment.
RunThisAsync recurses if WaitAsync succeeds but TryTake fails. Even if this is expected to be rare, the recursion is unbounded and can stack overflow or spin if the semaphore/bag ever get out of sync. This can be made safer and easier to reason about by replacing the recursion with a loop that re-waits (or retries TryTake) without growing the call stack.
…RunThisAsync Replace recursive self-call with a while loop sharing a single CancellationTokenSource so the 300-second timeout acts as a hard upper bound across all retries, preventing potential infinite loops if the semaphore/bag invariant is ever broken. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I wish you good luck in this endeavor. Just a word of warning: you should do some runtime estimates before engaging further in this work. This behavior is already possible with VsTest (via coverage capture options), but it is SLOWER than disabling coverage analysis altogether as soon as the code base as more than a couple of hundred tests in it. |
Summary
Implements per-test coverage capture for the Microsoft Testing Platform (MTP) test runner by running each test in an isolated server process.
Key changes:
SingleMicrosoftTestPlatformRunner.StopAndRemoveServerAsync()— stops server and removes from cache, triggering ProcessExit coverage flushSingleMicrosoftTestPlatformRunner.RunSingleTestForCoverageAsync()— runs one test, stops server, reads per-test coverage fileMicrosoftTestPlatformRunnerPool.CaptureCoverageTestByTest()— iterates all tests using the runner pool for parallelismCaptureCoverage()routing — uses per-test capture whenCoverageBasedTestflag is set, aggregate otherwiseNormalforperTest,ExactforperTestInIsolationWhy process restart: MTP doesn't have an in-process data collector like VsTest's
CoverageCollector. SinceMutantControlonly flushes coverage data onProcessExit, the most reliable way to get per-test coverage is to stop and restart the server between tests. This is a one-time cost during the coverage capture phase.Test plan
dotnet test src/Stryker.TestRunner.MicrosoftTestPlatform.UnitTest/(150 pass)dotnet build src/Stryker.slnx(0 errors)--coverage-analysis perTest --test-runner mtp--coverage-analysis perTestInIsolation --test-runner mtp--coverage-analysis all --test-runner mtp(aggregate fallback)Related