Repository navigation
[Test Optimization] Report sessions where VSTest ran no tests as skipped - #9486
tonyredondo wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
On Linux ARM64, the pinned .NET 11 RC2 VSTest process cannot share named mutexes with the net8.0–net10.0 integration-test host, causing the parent-session IPC assertions in VSTestEmptySessionTests to time out. The fix is to apply the SDK-aware guard already present in MsTestV2DiscoveryTests.RunDiscoveryScenario to the affected parent-session test inputs.
🤖 Bits Code Review · Commit 701a1b1
BenchmarksBenchmark execution time: 2026-10-23 16:11:50 Comparing candidate commit 854da7d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 72 metrics, 0 unstable metrics, 11 known flaky benchmarks, 115 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9486) and master. ✅ No regressions detected |
|
/azp run consolidated-pipeline |
50291ff to
c28d203
Compare
There was a problem hiding this comment.
More details
Zero-test classification preserves nonzero, unknown, and failed-run outcomes, with IPC drained before the final status decision. Runtime behavior was not exercised because the required .NET 11 RC SDK is unavailable.
🤖 Bits Code Review · Commit c28d203
1e14acd to
bf0693b
Compare
c28d203 to
dfe06c4
Compare
bf0693b to
a84c18a
Compare
dfe06c4 to
6eddb0a
Compare
When a VSTest run succeeds without running any test, the session is now reported with test.status:skip, test.skip_reason "No tests were executed" and test.session.empty_reason:zero_tests. A new integration on vstest.console's run completion handler sends the executed test count to the session that owns the run over the session IPC channel, so it works whether vstest.console or the dotnet CLI owns the session. The session is skipped only when every count it received is zero; an aborted, canceled or failed run, or a runner exception, keeps it passed.
… bug CI runs VSTest from the .NET 11 RC2 SDK, which can't share named mutexes with .NET 8-10 on Linux (dotnet/runtime#134491). In VSTestEmptySessionTests vstest.console couldn't write to the parent session's channel on x64 and ARM64, and on ARM64 the testhost couldn't open the session's channel. The SDK check moves from MsTestV2DiscoveryTests to TestingFrameworkEvpTest so both classes share it and it goes away in one place with .NET 11 GA. vstest.console runs on the SDK whatever the test's target framework, so the other frameworks still run these tests.
… test integrations The Coverlet and Microsoft CodeCoverage integrations each had a private copy of the lookup that extracts the propagated test session from the environment variables, DotnetCommon repeated it inline twice, and the VSTest run completion hook added another. They all use DotnetCommon.TryGetParentSessionSpanId now.
a84c18a to
94750e5
Compare
6eddb0a to
854da7d
Compare
Summary of changes
When VSTest runs no tests and the run succeeds, the test session is now reported as skipped:
test.status:skiptest.skip_reason:No tests were executedtest.session.empty_reason:zero_testsThis is what you get from a filter that matches nothing, with
dotnet test,dotnet vstestorvstest.console. Sessions that ran tests, and failed sessions, are reported as before. The tag, value and skip reason are the same as in the Go version (DataDog/dd-trace-go#5697) and datadog-ci-rb; dd-trace-js uses the same tag and value.This is an alternative to #9325. It's stacked on #9335, which fixes the IPC channel this relies on, so please review that one first.
Reason for change
An empty run that exits successfully was reported as
pass, so it looked the same as a session whose test events were lost.Implementation details
RunTestsArgumentExecutor+TestRunRequestEventsRegistrar.TestRunRequest_OnRunCompletion, the handler VSTest uses to print "No test matches the given testcase filter". Its signature hasn't changed from vstest 15.9 to today, and it only runs for command-line runs, so IDE (design mode) runs aren't affected. The count comes from VSTest itself, which means it includes tests from frameworks we don't instrument, and a session without test events can't be mistaken for an empty one. An aborted, canceled or failed run, or one without statistics, counts as unknown.SessionExecutedTestsMessageon the session's existing IPC channel. That session lives in vstest.console itself (dotnet vstest,vstest.console, anddotnet teston SDK 10 and later) or in thedotnetCLI process (dotnet teston SDK 9 and earlier). Only sessions created by the dotnet test integrations host the IPC server, so manual sessions never get a count.TestSessionkeeps one small state: nothing reported, every report was zero, or a runner ran tests or couldn't count them. A session that closes as passed with only zero reports closes as skipped. The check runs after the IPC server is disposed, so every report already sent has been read. A runner exception counts as unknown, since CallTarget passes exit code 0 in that case.CircularChannelfixes in [CI Visibility] Fix message loss and abandoned mutexes in the IPC channel #9335: a new client no longer drops the session's unread messages, the reader reads what was written before it's disposed, and an abandoned mutex no longer leaves the channel locked.DotnetCommonrepeated it inline twice, so it now lives once inDotnetCommon.TryGetParentSessionSpanId.Compared with #9325:
dotnet teston SDK 9 and earlier never got the new status.Microsoft.VisualStudio.TestPlatform.Client, a new target assembly that needs Honeypot and trimming entries, plus per-request tracking withConditionalWeakTables. This PR has one hook in vstest.console, which we already instrument.Test coverage
EmptyTestSessionTests: zero, non-zero, missing, aborted, canceled and failed runs, going through the real IPC path; zero reports from every runner; a failed session; a runner exception; a session without an IPC server. It mocksITestOptimizationto skip the 5 s agent discovery, so each test takes milliseconds.VSTestEmptySessionTests(integration, existing xUnit sample, real vstest.console): a filter that matches nothing gives a skipped session, and a matching test gives a passed one. With a parent session injected through environment variables, vstest.console sends 0 or 1 to it. Three of the four cases fail if the hook doesn't send the count. These tests skip for net8.0 to net10.0 on Linux and macOS while CI uses the .NET 11 RC SDK, because .NET 11 RC can't share named mutexes with .NET 8-10 (.NET 11 breaks named Mutex interop with every earlier .NET on Linux and macOS dotnet/runtime#134491). The SDK check thatMsTestV2DiscoveryTestsalready had now lives inTestingFrameworkEvpTestso both classes share it. vstest.console runs on the SDK, so the other target frameworks still cover the same code.dotnet testruns with the profiler attached: on SDK 9 thedotnetCLI session received the count and closed as skipped (0) or passed (1), and on SDK 10 the vstest.console session did the same.Datadog.Trace.Tests.Cinamespace, the new integration tests, and the Release build ofDatadog.Tracewith no warnings.Other details
dotnet test, and the VSTest projects run no tests while the MTP ones run some, the session can be reported as skipped, because MTP projects don't send a count./Tests:runs (RunSpecificTestsArgumentExecutor) aren't hooked and stay passed.