Skip to content

Add test coverage messages, coverage threshold checks, and summary#9896

Open
james-newell-forge wants to merge 11 commits into
microsoft:mainfrom
james-newell-forge:feat/add-coverage-messages
Open

Add test coverage messages, coverage threshold checks, and summary#9896
james-newell-forge wants to merge 11 commits into
microsoft:mainfrom
james-newell-forge:feat/add-coverage-messages

Conversation

@james-newell-forge

@james-newell-forge james-newell-forge commented Jul 13, 2026

Copy link
Copy Markdown

Summary

Adds first-class code coverage reporting to Microsoft.Testing.Platform (MTP). Coverage extensions can now publish coverage data and threshold results through the platform's data-consumer pipeline, which the terminal output device renders in the run summary and which drives a dedicated exit code on threshold failure.

Note that this PR was assisted with Claude Opus 4.8 model

Changes

New public API (Microsoft.Testing.Platform.Extensions.Messages)

  • TestCoverageMessage — reports coverage (ModuleName, Value, CoverageType) for a module.
  • TestCoverageThresholdMessage — reports a threshold evaluation (Value, Threshold, CoverageType, Status, Statistic).
  • Supporting enums: CoverageType (Line/Branch/Method), CoverageThresholdStatistic (Minimum/Total/Average), CoverageThresholdStatus (Passed/Failed).

Example

Test run summary: Passed! - bin/Debug/net10.0/tests.dll (net10.0|arm64)
  total: 22
  failed: 0
  succeeded: 22
  skipped: 0
  duration: 1s 288ms

  Out of process file artifacts produced:
    - /Users/james/test-project/tests/coverage.cobertura.xml

  Code Coverage Summary:
    Total - Line: 96.6%
    Total - Branch: 77.7%
    Total - Method: 91.7%

  Coverage Threshold Results:
    Line (Total): 96.6% >= 70.0% threshold
    Branch (Total): 77.7% >= 70.0% threshold
    Method (Total): 91.7% >= 70.0% threshold

Platform wiring

  • New internal ITestCoverageResult / TestCoverageResult data consumer, registered as a common service and consumer in both the framework and test-host-controller pipelines. It tracks coverage entries and threshold failures.
  • Coverage messages are consumed and rendered by TerminalOutputDevice, with a new AppendCoverageSummary in the terminal reporter that prints a Code Coverage Summary and color-coded Coverage Threshold Results (green pass / red fail). The summary is rendered from either the in-process test host or the out-of-process controller.
  • New exit code CoverageThresholdFailed = 14, returned from ConsoleTestHost and TestHostControllersTestHost when a threshold fails and the run would otherwise succeed.

Tests

  • Added TerminalTestReporter unit tests covering empty input, passing thresholds (>=), failing thresholds (<), and mixed pass/fail rendering.

Notes

  • New public API entries are declared in PublicAPI.Unshipped.txt; internal surface in InternalAPI.Unshipped.txt.
  • Changes would be need to be made to mimic the coverage output in the SDK (via IPC)

Copilot AI review requested due to automatic review settings July 13, 2026 10:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds first-class coverage messages, terminal summaries, threshold tracking, and threshold-specific exit handling to Microsoft.Testing.Platform.

Changes:

  • Introduces public coverage and threshold message contracts.
  • Registers consumers and renders coverage results in terminal summaries.
  • Returns exit code 14 when coverage thresholds fail.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/.../TerminalTestReporterTests.cs Tests coverage-summary rendering.
.../Services/TestCoverageResult.cs Tracks coverage and failed thresholds.
.../Services/ITestCoverageResult.cs Defines the internal result contract.
.../PublicAPI/PublicAPI.Unshipped.txt Declares new public APIs.
.../TerminalOutputDevice.SessionLifecycle.cs Renders coverage after sessions.
.../TerminalOutputDevice.DataConsumption.cs Consumes coverage messages.
.../TerminalOutputDevice.cs Registers message types and buffers.
.../TerminalTestReporter.Summary.cs Formats coverage summaries.
.../Messages/TestCoverageMessages.cs Adds coverage message contracts.
.../InternalAPI/InternalAPI.Unshipped.txt Records new internal APIs.
.../Hosts/TestHostControllersTestHost.cs Handles controller threshold exits.
.../Hosts/TestHostBuilder.Framework.cs Registers the framework consumer.
.../Hosts/TestHostBuilder.CommonServices.cs Registers the coverage service.
.../Hosts/ConsoleTestHost.cs Handles in-process threshold exits.
.../Helpers/ExitCodes.cs Adds exit code 14.
...Extensions.TrxReport/...Unshipped.txt Updates internal API baseline.
...Extensions.Retry/...Unshipped.txt Updates internal API baseline.
...Extensions.MSBuild/InternalAPI.Unshipped.txt Updates internal API baseline.
...Extensions.JUnitReport/...Unshipped.txt Updates internal API baseline.
...Extensions.HtmlReport/...Unshipped.txt Updates internal API baseline.
...Extensions.HangDump/...Unshipped.txt Updates internal API baseline.
...Extensions.GitHubActionsReport/...Unshipped.txt Updates internal API baseline.
...Extensions.CtrfReport/...Unshipped.txt Updates internal API baseline.
...Extensions.AzureDevOpsReport/...Unshipped.txt Updates internal API baseline.

Comment thread src/Platform/Microsoft.Testing.Platform/Hosts/ConsoleTestHost.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControllersTestHost.cs Outdated
@Evangelink

Copy link
Copy Markdown
Member

Hi @james-newell-forge — thanks for the contribution! I did a security pass and a fairly deep code review. Sharing findings below.

Prompt-injection / security check

No prompt injection found. The only AI-related text is the benign "this PR was assisted with Claude Opus 4.8" note in the description. Nothing in the diff attempts to manipulate reviewers or automation. 👍

Before going further: goal + tracking issue

A couple of process points before we dig into the code:

  1. What's the concrete goal here? Could you describe the end-to-end scenario you're trying to enable? Right now the PR adds message contracts, a consumer, terminal rendering, and a new exit code — but nothing in the codebase actually produces TestCoverageMessage / TestCoverageThresholdMessage. Your own note ("Changes would need to be made to mimic the coverage output in the SDK via IPC") confirms the producer side is missing, so this is effectively a public API + dead consumption path with no way to exercise it today. Knowing the real target scenario will help us judge the shape of the API.

  2. Please get an approved tracking issue first. This introduces a brand-new public API surface to Microsoft.Testing.Platform (two message types, three enums, a new exit code, a PublicAPI.Unshipped.txt contract). Per our contributing guidelines we want new features — especially new public API — to have an approved design/issue before implementation, since public API is essentially permanent. Could you open a Feature issue (use the native GitHub Feature issue type, not a type/feature label) describing the design so the team can sign off on the surface? That avoids you investing more effort in an API shape we might want to change.

I'll hold off requesting detailed changes until we align on the above, but here's the technical review so it's all in one place.

Design-level concerns

  • No producer / can't be tested end-to-end. Because nothing emits these messages, there are no acceptance/integration tests exercising the real path (terminal output + exit code 14). The added unit tests only call AppendCoverageSummary directly. This makes it hard to validate the feature actually works as a whole.
  • Duplicated state. Coverage entries are buffered twice: once in TerminalOutputDevice (_coverageEntries / _coverageThresholdEntries) and once in TestCoverageResult. Two independent consumers accumulating the same data is easy to get out of sync. Consider having the terminal device read from the single ITestCoverageResult service rather than maintaining its own lists.
  • Public API naming. TestCoverageThresholdMessage.Stat (of type CoverageThresholdStat) is an unclear abbreviation for a permanent public property. Something like Statistic / AggregationKind reads better. Same for the enum name.
  • PropertyBagData with an empty bag. Both messages derive from PropertyBagData but never populate the property bag — consumers that inspect the bag generically will see nothing. Is PropertyBagData the right base, or should these carry their data as bag properties?

Correctness / consistency issues

  • Missing localization (guideline violation). The user-facing terminal strings — "Code Coverage Summary:", "Coverage Threshold Results:", and the "Test coverage" / "Reports code coverage..." display names — are hardcoded English. Every other terminal summary string in TerminalTestReporter goes through TerminalResources.resx (e.g. TerminalResources.OutOfProcessArtifactsProduced). New strings must be added to the .resx and the .xlf files regenerated via /t:UpdateXlf.
  • Culture-sensitive number formatting. {entry.Value:F1} uses the current culture, so on some locales you'll get 96,6%. Terminal diagnostics elsewhere generally format with CultureInfo.InvariantCulture.
  • Thread safety. ConsumeAsync does an unguarded List.Add. Please confirm the data-consumer pump guarantees serialized calls per consumer; if not, these lists need synchronization.

Minor

  • The exit-code check is duplicated verbatim in ConsoleTestHost and TestHostControllersTestHost; a shared helper would avoid drift.
  • The 9 InternalAPI.Unshipped.txt edits across extension projects are just re-baselining the new ExitCode value — expected, but worth a sanity check that they were regenerated rather than hand-edited.

Happy to help once there's an approved issue and we know the target scenario. Thanks again!

Copilot AI review requested due to automatic review settings July 13, 2026 11:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.

Comment thread src/Platform/Microsoft.Testing.Platform/Hosts/ConsoleTestHost.cs Outdated
Comment thread src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControllersTestHost.cs Outdated

@Evangelink Evangelink left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking PR until my previous points are addressed

@microsoft-github-policy-service microsoft-github-policy-service Bot added the needs/author-feedback Waiting on the original author. label Jul 13, 2026
Copilot AI review requested due to automatic review settings July 13, 2026 12:08
@microsoft-github-policy-service microsoft-github-policy-service Bot added needs/attention Needs maintainer attention. and removed needs/author-feedback Waiting on the original author. labels Jul 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 4 comments.

@james-newell-forge

Copy link
Copy Markdown
Author

The goal on this change is to allow coverage extensions like Microsoft's coverage or coverlet to add threshold coverage checks (which would address microsoft/codecoverage#160). In addition, coverage summaries can be displayed as a first-class citizen. Will add a tracking issue.

@james-newell-forge

Copy link
Copy Markdown
Author

Thread safety. ConsumeAsync does an unguarded List.Add. Please confirm the data-consumer pump guarantees serialized calls per consumer; if not, these lists need synchronization.

TestCoverageResult.ConsumeAsync thread-safety

Verdict: List<T>.Add isn't thread-safe, but the usage here is — the message bus serializes ConsumeAsync per consumer.

Why it's safe

  • No concurrent Add. Each IDataConsumer gets one AsyncConsumerDataProcessor with a single-reader channel (SingleReader = true) and one background loop that awaits each ConsumeAsync before reading the next. Many threads may PublishAsync, but dispatch is serialized → two .Adds never overlap.
  • Safe reads. All readers run only after DrainDataAsync() (+ DisableAsync()):
    • the hosts read HasCoverageThresholdFailure for the exit code, and
    • TerminalOutputDevice.DisplayAfterSessionEndRunAsync reads CoverageEntries / ThresholdEntries to render the summary.
      The drain marker (awaited TaskCompletionSource) is a happens-before barrier, so all writes are visible and no read races an in-flight Add.

Caveats

  • Relies on the contract that only the bus calls ConsumeAsync (true here).
  • Would race if any code read the collections before the drain (none does today — both the host exit-code check and the terminal summary run post-drain).

Recommendation

No change needed — consistent with the platform's other built-in consumers.

Copilot AI review requested due to automatic review settings July 13, 2026 12:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.

Comment on lines +17 to +19
/// Shared by <c>ConsoleTestHost</c> (in-process) and <c>TestHostControllersTestHost</c> (out-of-process)
/// so the two exit-code paths can't drift apart. Both call this only after their own exit code has been
/// finalized (and, for the controller, after the child's ignore-exit-code policy has been applied).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional. We're treating coverage as always produced out-of-process: any coverage extension registers as an ITestHostProcessLifetimeHandler, which forces test-host-controller mode, so ConsoleTestHost runs as the child and never receives coverage messages into its (empty) ITestCoverageResult. The threshold verdict is therefore owned solely by the controller (TestHostControllersTestHost), which registers the coverage consumer, drains the queue after the child exits, applies exit code 14, and routes it through --ignore-exit-code — all covered end-to-end by CoverageThresholdControllerExitCodeTests.

Because in-process coverage production isn't a supported scenario, applying the policy in ConsoleTestHost would be dead code, and an in-process acceptance test would assert a path that can't occur. That's why the in-process test and the ConsoleTestHost override were removed rather than kept. If we decide to support in-process coverage collectors later, we'll reintroduce both together.

Copilot AI review requested due to automatic review settings July 14, 2026 14:31
@james-newell-forge james-newell-forge force-pushed the feat/add-coverage-messages branch from 427501c to 7ddfc77 Compare July 14, 2026 14:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated 4 comments.

@james-newell-forge

Copy link
Copy Markdown
Author

Please get an approved tracking issue first. This introduces a brand-new public API surface to Microsoft.Testing.Platform (two message types, three enums, a new exit code, a PublicAPI.Unshipped.txt contract). Per our contributing guidelines we want new features — especially new public API — to have an approved design/issue before implementation, since public API is essentially permanent. Could you open a Feature issue (use the native GitHub Feature issue type, not a type/feature label) describing the design so the team can sign off on the surface? That avoids you investing more effort in an API shape we might want to change.

Tracking Issue created: #9943

Copilot AI review requested due to automatic review settings July 14, 2026 14:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.

foreach (TestCoverageMessage entry in coverageEntries)
{
terminal.Append(DoubleIndentation);
terminal.AppendLine($"{entry.ModuleName} - {GetCoverageTypeLabel(entry.CoverageType)}: {entry.Value.ToString("F1", CultureInfo.CurrentCulture)}%");
Comment on lines +147 to +148
/// <param name="value">The actual coverage value.</param>
/// <param name="threshold">The required threshold value.</param>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs/attention Needs maintainer attention.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants