Remove Windows-Review from cross-platform tests - #16113
Merged
Jakub Jareš (nohwnd) merged 8 commits intoJun 29, 2026
Merged
Conversation
Remove [TestCategory("Windows-Review")] from ~45 tests that don't use
Windows-specific APIs or features. These tests were incorrectly marked as
Windows-only.
Changed test files (22 files):
- DotnetTestMSBuildOutputTests.cs (3 tests) - use InvokeDotnetTest
- LoggerTests.cs (6 tests) - TRX/HTML logger validation
- SerializationCompatibilityTests.cs (4 tests) - JSON serialization
- ExecutionTests.cs (7 of 10 tests) - cross-platform execution
- ArgumentProcessorTests.cs (3 tests) - CLI validation
- DiscoveryTests.cs (2 tests) - test discovery
- FrameworkTests.cs (2 tests) - framework selection
- TranslationLayerTests (8 tests across 8 files) - API tests
- vstest.console.UnitTests (4 tests across 3 files) - unit tests
- And 3 more single-test files
Kept Windows-Review on legitimately platform-specific tests:
- ExecutionTests.cs: 3 x86/x64 architecture-specific tests
- All EventLogCollectorTests, BlameDataCollectorTests, etc.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the TestCategory("Windows-Review") filter from a broad set of unit, integration, and acceptance tests so they can run on non-Windows agents when they don’t rely on Windows-only APIs or tooling.
Changes:
- Removed
Windows-Reviewcategorization across multiple unit tests and end-to-end test suites (translation-layer, acceptance/integration). - Enabled more
dotnet test/ logger / runsettings / discovery / filtering scenarios to execute cross-platform. - Kept/left Windows-only coverage on tests that still require Windows-specific features (per PR description).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| test/vstest.console.UnitTests/Processors/ShowDeprecateDotnetVStestMessageArgumentProcessorTests.cs | Removes Windows-only categorization from CLI processor unit tests. |
| test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs | Removes Windows-only categorization from blame processor unit tests. |
| test/vstest.console.UnitTests/Processors/AeDebuggerArgumentProcessorTest.cs | Removes Windows-only categorization from AeDebugger processor unit tests. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/TargetFrameworkTestHostDemultiplexer.cs | Removes Windows-only categorization from translation-layer demultiplexer coverage. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/SerializeTestRunTests.cs | Removes Windows-only categorization from translation-layer serialization coverage. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/RunTestsWithFilterTests.cs | Removes Windows-only categorization from translation-layer filter test (compat matrix-backed). |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/RunTests.cs | Removes Windows-only categorization from translation-layer run tests (compat matrix-backed). |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DiscoverTests.cs | Removes Windows-only categorization from translation-layer discovery tests (compat matrix-backed). |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DifferentTestFrameworkSimpleTests.cs | Removes Windows-only categorization from translation-layer adapter tests. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DataCollectorAttachmentProcessor.cs | Removes Windows-only categorization from translation-layer attachments processing test. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/CustomTestHostLauncherTests.cs | Removes Windows-only categorization from custom testhost launcher tests (compat matrix-backed). |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TestCaseFilterTests.cs | Removes Windows-only categorization from acceptance filter coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/SerializationCompatibilityTests.cs | Removes Windows-only categorization from STJ/Newtonsoft compatibility coverage (compat matrix-backed). |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/RunsettingsTests.cs | Removes Windows-only categorization from runsettings acceptance coverage (incl. dotnet-test project scenario). |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MultitargetingTestHostTests.cs | Removes Windows-only categorization from multi-targeting acceptance coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/LoggerTests.cs | Removes Windows-only categorization from TRX/HTML logger acceptance coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/FrameworkTests.cs | Removes Windows-only categorization from framework selection acceptance coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ExecutionTests.cs | Removes Windows-only categorization from multiple execution acceptance tests (compat matrix-backed). |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetTestMSBuildOutputTests.cs | Removes Windows-only categorization from dotnet test MSBuild-output acceptance coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DiscoveryTests.cs | Removes Windows-only categorization from acceptance discovery coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs | Removes Windows-only categorization from acceptance data collection coverage. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ArgumentProcessorTests.cs | Removes Windows-only categorization from acceptance argument-processor coverage. |
Comments suppressed due to low confidence (1)
test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/DataCollectorAttachmentProcessor.cs:29
- The wrapper is created in the constructor before
SetTestEnvironment(_testEnvironment, runnerInfo)runs, so on Linux/macOS it will use the defaultnet481desktop runner and attempt to startvstest.console.exe(Windows-only). If this test is meant to run cross-platform, delay creating the wrapper until afterSetTestEnvironmentso the correct runner is used.
public class DataCollectorAttachmentProcessor : AcceptanceTestBase
{
private readonly IVsTestConsoleWrapper _vstestConsoleWrapper;
private readonly RunEventHandler _runEventHandler;
private readonly TestRunAttachmentsProcessingEventHandler _testRunAttachmentsProcessingEventHandler;
This was referenced Jun 13, 2026
AeDebug is a Windows-only feature and these tests rely on Windows path semantics (c:\...), so they fail on Linux/macOS. Restore the category that was wrongly removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… runner These tests use the compatibility/wrapper matrix data sources (Runner/TestHost/MSTest/WrapperCompatibilityDataSource) which emit .NET Framework runner+testhost combinations on every OS, unlike NetFullTargetFrameworkDataSource which self-gates net48 rows behind isWindows. Without Windows-Review they run on Linux/macOS where the desktop runner doesn't exist and fail (143 row-failures across the two integration projects). Also re-gated two net(core) tests that genuinely differ on non-Windows: the MSBuildLogger special-char round-trip and the framework-incompatible warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The xUnit adapter returns no results on Linux (the diagnostic log shows a NullReferenceException because path is null), so the run yields an empty sequence and .First() throws 'Sequence contains no elements'. It passed on macOS and on an earlier ubuntu run, so it's flaky, but the original Windows-Review gating was correct. Restored it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ConsiderEmptyDataSourceAsInconclusive made MSTest treat a data source that produces no rows as inconclusive instead of failing. NetFullTargetFrameworkDataSource produces 0 rows on non-Windows, so dropping Windows-Review from those tests did not make them run cross-platform - it made them silently pass as inconclusive. Remove the setting from both integration test .runsettings so an empty data source fails loudly again. Move the tests that are genuinely cross-platform to the NetCore testhost so they produce rows on every platform: argument processor help, execution exit codes, runtime provider discovery and execution, trx and html loggers, and fully qualified discovery. Put Windows-Review back on the tests that really need the .NET Framework runner: MSTest v1 adapter, non-dll adapter, and full framework assembly loading. Also mark the pre-existing NetFull-only tests that the setting was masking (CreateNoNewWindow, LiveUnitTesting) as Windows-Review, otherwise they would start failing on Linux now that empty data sources are no longer hidden. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removing ConsiderEmptyDataSourceAsInconclusive makes MSTest fail on an empty data source instead of silently going inconclusive. That exposes every test whose data sources yield zero rows on Linux/macOS, not only the plain NetFull ones from the first pass. A second sweep of both integration test projects covering all four OS-empty vectors - NetFullTargetFrameworkDataSource, NetFrameworkRunner, NetCoreRunner with net4x-only TFMs (NETFX = net481), and NetCoreTargetFrameworkDataSource(useCoreRunner: false) - turned up four more genuinely Framework-bound tests: - MultitargetingTestHostTests: multitargets a net481 testhost - SerializerSelectionTests.OnNetFrameworkRunner_ShouldUseJsonite: the Framework runner's Jsonite serializer; the core side is already covered - FrameworkTests.OnWrongFrameworkPassedTestRunShouldNotRun: the only assertion runs on the desktop runner - RecursiveResourcesLookupTests: mscorlib resource crash repro, .NET Framework only (and currently ignored) None of these run cross-platform, so categorize them rather than move them to the core testhost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSTest validates each ITestDataSource on a method individually, so a test carrying both NetFullTargetFrameworkDataSource and NetCoreTargetFrameworkDataSource fails on Linux/macOS once the empty-data masking is gone: NetFull produces no rows off Windows, and that empty source is now a hard failure rather than inconclusive. Drop the redundant NetFull source from the tests that don't validate .NET Framework testhost behaviour - their .NET (net11.0) rows already ran on Linux. Keep NetFull and mark Windows-Review on the four tests that assert .NET Framework-specific behaviour (stack overflow / unhandled exception messages, and source navigation across both testhosts), since that coverage genuinely needs the Windows-only .NET Framework testhost. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jakub Jareš (nohwnd)
enabled auto-merge (squash)
June 29, 2026 12:54
Artur Spychaj (drognanar)
approved these changes
Jun 29, 2026
Jakub Jareš (nohwnd)
added a commit
to nohwnd/vstest
that referenced
this pull request
Jun 29, 2026
Resolve conflicts from microsoft#16113 (Remove Windows-Review from cross-platform tests). Took origin/main as the semantic baseline for testhost selection and Windows-Review placement, then translated the old data-source attributes to the new [TestMatrix]/[CompatibilityMatrix] vocabulary so both PRs' intents are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
~45 tests were marked
[TestCategory("Windows-Review")]but don't use any Windows-specific APIs. Removed the restriction from:InvokeDotnetTest— the patched dotnet works on all platforms nowLeft Windows-Review on tests that genuinely need it (EventLog, Blame/procdump, code coverage, STA threading, native C++, etc.).