Skip to content

Move to mstest 4#15443

Merged
Evangelink merged 8 commits into
microsoft:mainfrom
nohwnd:move-to-mstest4
Mar 9, 2026
Merged

Move to mstest 4#15443
Evangelink merged 8 commits into
microsoft:mainfrom
nohwnd:move-to-mstest4

Conversation

@nohwnd
Copy link
Copy Markdown
Member

@nohwnd nohwnd commented Mar 6, 2026

Description

Moved to mstest 4.1, some edits were a bit annoying, with the formatting no longer working for assertions. Also the context moved to be the same directory as deploy, which caused a nice message saying that file is in use, but in reality I was saying

var destination = samePath;
Copy(samePath, destination)

Fixed unit tests for communication utilities, to no longer reset the timeout to 90s when running in parallel, because then few tests that are unlucky will always run into it and wait 90s for timeout they test.

Test.cmd before: 09m19s
Test.cmd after: 0m35s

@nohwnd nohwnd requested a review from Evangelink as a code owner March 6, 2026 22:28
Copilot AI review requested due to automatic review settings March 6, 2026 22:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the test suite to MSTest 4.1 and adjusts tests/utilities to work with updated analyzers/behaviors, including improvements to parallel test execution stability and runtime.

Changes:

  • Migrated many tests from Assert.ThrowsException / ExpectedException to Assert.ThrowsExactly (and async variants), plus assertion message formatting updates.
  • Adjusted data-driven tests/attributes and added targeted DoNotParallelize to avoid cross-test interference from global environment variables.
  • Updated test infrastructure and package versions (MSTest 4.1, Microsoft Testing Platform, CodeCoverage) and added helpers for environment-variable scoping.

Reviewed changes

Copilot reviewed 113 out of 113 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/vstest.console.UnitTests/TestPlatformHelpers/TestRequestManagerTests.cs Switch exception assertions to ThrowsExactly; adjust data-driven attributes.
test/vstest.console.UnitTests/Processors/Utilities/ArgumentProcessorFactoryTests.cs Update Assert.IsNotNull message formatting for MSTest 4.
test/vstest.console.UnitTests/Processors/UseVsixExtensionsArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/TestSessionCorrelationIdProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/ShowDeprecateDotnetVStestMessageArgumentProcessorTests.cs Suppress MSTest analyzer warning for constant assertion.
test/vstest.console.UnitTests/Processors/RunTestsArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/RunSpecificTestsArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/PortArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/ListTestsArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/ListFullyQualifiedTestsArgumentProcessorTests.cs Replace ExpectedException with explicit ThrowsExactly assertions.
test/vstest.console.UnitTests/Processors/EnableDiagArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs Replace ExpectedException with explicit ThrowsExactly assertion.
test/vstest.console.UnitTests/Processors/DisableAutoFakesArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/CollectArgumentProcessorTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Processors/CLIRunSettingsArgumentProcessorTests.cs Update DynamicData usage and exception assertions for MSTest 4.
test/vstest.console.UnitTests/Processors/ArtifactProcessingPostProcessModeProcessorTest.cs Switch ctor-guard tests to ThrowsExactly.
test/vstest.console.UnitTests/Processors/ArtifactProcessingCollectModeProcessorTest.cs Switch ctor-guard tests to ThrowsExactly.
test/vstest.console.UnitTests/Processors/AeDebuggerArgumentProcessorTest.cs Switch to ThrowsExactly; suppress constant-assert analyzer warning.
test/vstest.console.UnitTests/Internal/FilePatternParserTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs Switch to ThrowsExactly; adjust data-driven method attributes.
test/vstest.console.UnitTests/InProcessVsTestConsoleWrapperTests.cs Switch to ThrowsExactly for exception assertions.
test/vstest.console.UnitTests/CommandLine/CommandLineOptionsTests.cs Suppress MSTest analyzer warning for constant assertion; switch to ThrowsExactly.
test/testhost.UnitTests/UnitTestClientTests.cs Switch to ThrowsExactly for exception assertions.
test/testhost.UnitTests/TestHostTraceListenerTests.cs Replace ExpectedException with ThrowsExactly assertions.
test/testhost.UnitTests/DefaultEngineInvokerTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/TestPlatformDataCollectionSinkTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/TestPlatformDataCollectionLoggerTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/TestPlatformDataCollectionEventsTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/DataCollectorMainTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/DataCollectorConfigTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/DataCollectionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/DataCollectionEnvironmentVariableTests.cs Switch to ThrowsExactly for exception assertions.
test/datacollector.UnitTests/DataCollectionAttachmentManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/TranslationLayer.UnitTests/VsTestConsoleWrapperTests.cs Switch to ThrowsExactly for exception assertions.
test/TranslationLayer.UnitTests/VsTestConsoleRequestSenderTests.cs Normalize Assert.AreEqual argument order in callbacks.
test/TranslationLayer.UnitTests/DiscoveryEventsHandleConverterTests.cs Switch to ThrowsExactly for exception assertions.
test/TranslationLayer.UnitTests/ConsoleParametersTests.cs Normalize Assert.AreEqual argument order.
test/SettingsMigrator.UnitTests/MigratorTests.cs Replace ExpectedException with explicit exception assertions and add cleanup logic.
test/Microsoft.TestPlatform.Utilities.UnitTests/XmlUtilitiesTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Utilities.UnitTests/InferRunSettingsHelperTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.Utilities.UnitTests/CodeCoverageDataAttachmentsHandlerTests.cs Fix deployment/assembly path copy logic; switch to ThrowsExactlyAsync.
test/Microsoft.TestPlatform.Utilities.UnitTests/ClientUtilitiesTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs Improve assertion failure diagnostics using formatted messages.
test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.ObjectModel.UnitTests/Utilities/XmlRunSettingsUtilitiesTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.ObjectModel.UnitTests/Utilities/FilterHelperTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.ObjectModel.UnitTests/RunSettings/RunConfigurationTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests/TrxLoggerTests.cs Switch to ThrowsExactly and normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs Switch to ThrowsExactly and normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/XmlReaderWriterTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameLoggerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/TestLoggerManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/PostProcessing/ArtifactProcessingTests.cs Switch to ThrowsExactlyAsync for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Execution/BaseRunTestsTests.cs Normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs Normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/DataCollection/ProxyDataCollectionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/DataCollection/InProcDataCollectionSinkTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyTestSessionManagerTests.cs Switch to ThrowsExactly and normalize boolean asserts.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyOperationManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyExecutionManagerWithDataCollectionTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyExecutionManagerTests.cs Switch to ThrowsExactly for exception assertions and messages.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs Update failure messages to interpolation for MSTest 4 formatting.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyDiscoveryManagerTests.cs Update failure messages to interpolation for MSTest 4 formatting.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/DiscoveryDataAggregatorTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/AttachmentsProcessing/TestRunAttachmentsProcessingManagerTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/AttachmentsProcessing/DataCollectorAttachmentProcessorAppDomainTests.cs Switch to ThrowsExactlyAsync for cancellation tests.
test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Adapter/FrameworkHandleTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CoreUtilities.UnitTests/Utilities/JobQueueTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CoreUtilities.UnitTests/Tracing/EqtTraceTests.cs Update assertion message formatting for MSTest 4.
test/Microsoft.TestPlatform.CoreUtilities.UnitTests/Helpers/DotnetHostHelperTest.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/TestRequestSenderTests.cs Remove cleanup resetting env var; add DoNotParallelize where env var is mutated.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/Serialization/TestResultSerializationTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs Add disposable helper for scoped environment variable overrides.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionTestCaseEventSenderTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionTestCaseEventHandlerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionRequestHandlerTests.cs Add DoNotParallelize + scoped env var override via helper for timeout tests.
test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/AssemblyInitializer.cs Add assembly-wide env var initialization to reduce timeout flakiness.
test/Microsoft.TestPlatform.CommunicationUtilities.Platform.UnitTests/SocketServerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.CommunicationUtilities.Platform.UnitTests/SocketCommunicationManagerTests.cs Switch to ThrowsExactly; remove redundant Assert.IsTrue(true).
test/Microsoft.TestPlatform.CommunicationUtilities.Platform.UnitTests/SocketClientTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/Utilities/RunSettingsUtilitiesTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/Utilities/FakesUtilitiesTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/SettingsProvider/SettingsProviderExtensionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/RunSettingsTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/RunSettingsManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/RequestDataTests.cs Replace ExpectedException with explicit ThrowsExactly for property setter guards.
test/Microsoft.TestPlatform.Common.UnitTests/Logging/InternalTestLoggerEventsTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/Hosting/TestHostExtensionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/Filtering/FilterExpressionTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/Filtering/FastFilterTests.cs Normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.Common.UnitTests/Filtering/ConditionTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/Utilities/RunSettingsProviderExtensionsTests.cs Switch to ThrowsExactly and normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/TestPluginManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/TestPluginCacheTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/TestLoggerExtensionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/TestExtensionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Common.UnitTests/ExtensionFramework/DataCollectorExtensionManagerTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Client.UnitTests/TestPlatformTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Client.UnitTests/Execution/TestRunRequestTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Client.UnitTests/Discovery/DiscoveryRequestTests.cs Switch to ThrowsExactly; adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.Client.UnitTests/DesignMode/DesignModeClientTests.cs Replace ExpectedException with explicit ThrowsExactly assertions.
test/Microsoft.TestPlatform.AdapterUtilities.UnitTests/ManagedNameUtilities/ManagedNameParserTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TranslationLayerTests/SerializeTestRunTests.cs Switch to ThrowsExactly for exception assertions.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TelemetryTests.cs Update failure message formatting for MSTest 4.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/PostProcessingTests.cs Normalize Assert.AreEqual argument order.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectorAttachmentsProcessorsFactoryTests.cs Adjust data-driven test attribute usage.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/AppDomainTests.cs Improve assertion diagnostics using formatted messages.
test/DataCollectors/Microsoft.TestPlatform.Extensions.EventLogCollector.UnitTests/EventLogDataCollectorTests.cs Switch to ThrowsExactly for exception assertions.
eng/Versions.props Bump MSTest, Microsoft Testing Platform, and CodeCoverage package versions.
Comments suppressed due to low confidence (10)

test/SettingsMigrator.UnitTests/MigratorTests.cs:1

  • The finally cleanup deletes _oldRunsettingsPath, but this test creates _oldTestsettingsPath (and writes _newRunsettingsPath). As written, the temp file created for the test can be left behind and the intended file might never be deleted. Delete the correct paths (at least _oldTestsettingsPath, and consider cleaning _newRunsettingsPath if it is a temp file for this test).
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/AssemblyInitializer.cs:1
  • This sets a process-wide environment variable for the entire test run and never restores the previous value. Consider capturing the original env-var value in AssemblyInitialize and resetting it in an [AssemblyCleanup], or using the new disposable helper to scope changes, to avoid cross-assembly side effects when tests share a process.
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs:1
  • Consider renaming ResetableEnvironment to ResettableEnvironment (common spelling), and DisposableVariable to something more specific like DisposableEnvironmentVariable, to make intent clearer at call sites and improve discoverability.
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs:1
  • Consider renaming ResetableEnvironment to ResettableEnvironment (common spelling), and DisposableVariable to something more specific like DisposableEnvironmentVariable, to make intent clearer at call sites and improve discoverability.
    test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs:1
  • Typo in assertion message: 'Excepted' should be 'Expected'.
    test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs:1
  • The dangling semicolon on its own line makes this assertion block harder to read and looks like an accidental formatting artifact. Inline the semicolon at the end of the previous line (or remove the line break) to keep the statement idiomatic.
    test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyExecutionManagerTests.cs:1
  • Typo in failure message string: 'Concurrreny' should be 'Concurrency' (helps when grepping logs and keeps error output professional).
    test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/Parallel/ParallelProxyDiscoveryManagerTests.cs:1
  • Consider keeping spelling consistent across parallel-manager tests: other updated messages still use 'Concurrreny'. Standardize on 'Concurrency' everywhere for consistency in logs/searchability.
    test/vstest.console.UnitTests/Processors/CLIRunSettingsArgumentProcessorTests.cs:1
  • For DynamicData, consider keeping the data source type explicit (e.g., DynamicDataSourceType.Method / Property) if the member is not obviously a property. This reduces ambiguity for readers and helps avoid subtle behavior differences across MSTest versions/analyzers when updating again.
    test/vstest.console.UnitTests/Processors/ShowDeprecateDotnetVStestMessageArgumentProcessorTests.cs:1
  • The analyzer warning indicates this assertion is effectively a compile-time tautology (likely because CommandName is a const). Consider removing this test (since the compiler already enforces the const value), or changing the production value to be non-const (e.g., static readonly) if you specifically want a runtime regression test—either way avoids needing the pragma suppression.

@nohwnd
Copy link
Copy Markdown
Member Author

nohwnd commented Mar 7, 2026

Speed of unit tests seems capped by single project:

Test run summary: Passed! - D:\a_work\1\s\artifacts\bin\vstest.console.UnitTests\Release\net48\vstest.console.UnitTests.exe (.NET Framework 4.8|x64)
total: 649
failed: 0
succeeded: 647
skipped: 2
duration: 34s 408ms

…by calling e.g. DiscoverTests, but we never called EndSession, so it is just eating memory
Copilot AI review requested due to automatic review settings March 7, 2026 09:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 133 out of 133 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (7)

test/SettingsMigrator.UnitTests/MigratorTests.cs:1

  • The cleanup in finally deletes _oldRunsettingsPath, but this test creates _oldTestsettingsPath (and writes _newRunsettingsPath). This likely leaves temp files behind and may delete the wrong file. Update the finally block to delete the paths actually created in this test (at minimum _oldTestsettingsPath, and consider _newRunsettingsPath if it’s meant to be disposable).
    test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DotnetTestHostManagerTests.cs:1
  • Calling .Wait() on a canceled/faulted Task typically throws AggregateException (wrapping OperationCanceledException), so this assertion is likely incorrect/flaky. Prefer an async test and await Assert.ThrowsExactlyAsync<OperationCanceledException>(...), or assert AggregateException and validate InnerException is OperationCanceledException.
    test/Microsoft.TestPlatform.TestHostProvider.UnitTests/Hosting/DefaultTestHostManagerTests.cs:1
  • Same issue as above: .Wait() commonly throws AggregateException rather than OperationCanceledException directly. Convert the test to async + ThrowsExactlyAsync, or assert AggregateException and verify the inner exception.
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs:1
  • Typo in the helper name: ResetableEnvironment is typically spelled ResettableEnvironment (with double 't'). Renaming (and similarly considering DisposableVariable -> something like EnvironmentVariableScope) would make intent clearer and avoid propagating the misspelling.
    test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs:1
  • This test previously included validations after initialization (e.g., warning output and runsettings content). With Assert.ThrowsExactly(...) you can still verify those side effects by capturing the exception and performing assertions after the call, but the current version only checks that an exception is thrown. If the side effects are still expected behavior, re-add the assertions to avoid losing coverage.
    test/vstest.console.UnitTests/Processors/EnableBlameArgumentProcessorTests.cs:1
  • This test previously included validations after initialization (e.g., warning output and runsettings content). With Assert.ThrowsExactly(...) you can still verify those side effects by capturing the exception and performing assertions after the call, but the current version only checks that an exception is thrown. If the side effects are still expected behavior, re-add the assertions to avoid losing coverage.
    test/Microsoft.TestPlatform.Utilities.UnitTests/CodeCoverageDataAttachmentsHandlerTests.cs:1
  • Comparing DirectoryInfo(...).FullName via != can mis-detect equality on Windows when paths differ only by case (or other normalization), which may reintroduce the 'copy onto itself' scenario this code is trying to avoid. Consider comparing with OS-appropriate semantics (e.g., StringComparer.OrdinalIgnoreCase on Windows, Ordinal on Unix) after normalizing with Path.GetFullPath / Path.TrimEndingDirectorySeparator.

Copilot AI review requested due to automatic review settings March 7, 2026 12:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 134 out of 134 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (6)

test/vstest.console.UnitTests/TestPlatformHelpers/TestRequestManagerTests.cs:1

  • This method is still parameterized and uses DataRow, but the attribute was changed from [DataTestMethod] to [TestMethod]. In MSTest, [TestMethod] does not support parameters/data rows, so this will typically fail discovery/execution for these tests. Switch back to [DataTestMethod] (and likewise for other similar DataRow/DynamicData conversions in this PR).
    test/vstest.console.UnitTests/Processors/CLIRunSettingsArgumentProcessorTests.cs:1
  • This test still has parameters and uses [DynamicData], but was changed to [TestMethod]. Unless MSTest 4 explicitly supports parameterized [TestMethod] (historically it does not), this will break test discovery. Prefer [DataTestMethod] for parameterized tests driven by DynamicData.
    test/SettingsMigrator.UnitTests/MigratorTests.cs:1
  • The finally cleanup deletes _oldRunsettingsPath, but this test creates/writes _oldTestsettingsPath (and _newRunsettingsPath). This looks like a variable mix-up and may leave temp files behind (or delete the wrong file). Clean up the paths that are actually created in this test (likely _oldTestsettingsPath and possibly _newRunsettingsPath).
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs:1
  • Class name ResetableEnvironment is likely meant to be ResettableEnvironment (standard spelling). Renaming will improve readability and avoid propagated typos across usages.
    test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/ResetableEnvironment.cs:1
  • Class name ResetableEnvironment is likely meant to be ResettableEnvironment (standard spelling). Renaming will improve readability and avoid propagated typos across usages.
    test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBase.cs:1
  • Correct the typo in the assertion message string ('Excepted' -> 'Expected') to avoid confusing output when failures occur.

.Returns(PlatformArchitecture.X64);

_executor.Initialize(invalidString);
_mockOutput.Verify(x => x.WriteLine(string.Format(CultureInfo.CurrentCulture, CommandLineResources.InvalidBlameArgument, invalidString), OutputLevel.Warning));
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

removed because the code below did not run in the test, since it throws in the _executor.Initialize method and expects that exception

public void ShowDeprecateDotnetVStestMessageProcessorCommandName()
{
#pragma warning disable MSTEST0032 // Assertion condition is always true
Assert.AreEqual("/ShowDeprecateDotnetVSTestMessage", ShowDeprecateDotnetVStestMessageArgumentProcessor.CommandName);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah this is kinda weird, how do I ensure that constant has the given value, maybe you already fixed it in your previews

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

similar issue in other place with comparing a long constant with a 1000 int literal. I get warnings that the assertion is never true, and when I change it to say 1000l to force the long type, I get warning that the assert is never false.

@Evangelink Evangelink enabled auto-merge (squash) March 7, 2026 12:30
Copy link
Copy Markdown
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

You could consider using the metapackage versions for MSTest

@nohwnd
Copy link
Copy Markdown
Member Author

nohwnd commented Mar 7, 2026

Acceptance tests not parallelized in (on project level) in CI anymore? Projects seem to run one by one, we are using arcade mtp integration

@nohwnd
Copy link
Copy Markdown
Member Author

nohwnd commented Mar 7, 2026

You could consider using the metapackage versions for MSTest

Yes, I plan to change it, just not right here, because I don't know if we have some assumptions around that. Maybe no, but we plan to reduce the runtime of acceptance tests so I am postponing nice-to-have changes after that.

@nohwnd
Copy link
Copy Markdown
Member Author

nohwnd commented Mar 8, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants