Migrate Microsoft.DotNet.HotReload.Client.Tests to MSTest.Sdk#54731
Conversation
- Switch SDK to MSTest.Sdk; set UseMSTestSdk=true to opt out of test/Directory.Build.targets xUnit defaults. - Replace [Fact]/[Theory]/[InlineData] with [TestMethod]/[DataRow]. - Replace Xunit.Combinatorial with Combinatorial.MSTest 2.0.0. - Replace ITestOutputHelper with TestContext. - Inline project-local helpers (or replace AssertEx.X with MSTest equivalents). - Preserve multi-TFM (net11.0 + net472). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The package was added speculatively during the xUnit -> MSTest.Sdk migration but is not used by any test in this project. It is also not available on dnceng's internal NuGet feeds (only on nuget.org), which breaks CI restore with NU1101. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Helix work-item command was hard-coded to 'dotnet test <dll>' with
VSTest-style options (--filter, --logger trx, --blame-hang*). MSTest.Sdk
projects in this repo are Microsoft.Testing.Platform (MTP) executables
and do not include testhost.dll, so the VSTest invocation aborted with:
An assembly specified in the application dependencies manifest
(testhost.deps.json) was not found: package: 'testhost' ...
XUnitPublish.targets now exposes GetIsMTPProject which returns the
project's UseMSTestSdk value, XUnitRunner.targets stamps it onto
SDKCustomXUnitProject items as IsMTPProject metadata, and the
SDKCustomCreateXUnitWorkItemsWithTestExclusion task switches to
'dotnet exec <dll>' with MTP-native CLI when that metadata is set:
--filter (same MSTest syntax)
--results-directory (same)
--report-trx replaces '--logger trx'
--diagnostic* replaces VSTest '-d <log>'
xUnit v3 projects continue to use the existing VSTest command path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Migrates Microsoft.DotNet.HotReload.Client.Tests from xUnit to MSTest.Sdk (Microsoft.Testing.Platform-based), and updates the repo’s Helix/xunit-runner infrastructure to detect and invoke MTP-based test projects appropriately while preserving the project’s net11.0 + net472 multi-targeting.
Changes:
- Converted HotReload client tests from xUnit attributes/assertions to MSTest equivalents and introduced a project-local
TestLogger. - Extended
test/xunit-runner+ Helix work item generation to detect MTP projects (UseMSTestSdk=true) and switch invocation to an MTP-native command line. - Added
MSTest.SdkMSBuild SDK pinning inglobal.jsonand updated dependabot-tracked test dependencies.
Show a summary per file
| File | Description |
|---|---|
| test/xunit-runner/XUnitRunner.targets | Adds MSBuild-time detection of MTP projects and flows it as metadata to Helix work item creation. |
| test/xunit-runner/XUnitPublish.targets | Adds GetIsMTPProject target to expose UseMSTestSdk for the runner infrastructure. |
| test/Microsoft.DotNet.HotReload.Client.Tests/Utilities/TestLogger.cs | Adds a project-local logger compatible with MSTest TestContext output. |
| test/Microsoft.DotNet.HotReload.Client.Tests/StaticWebAssetsManifestTests.cs | Migrates tests to MSTest attributes/assert APIs and wires MSTest TestContext. |
| test/Microsoft.DotNet.HotReload.Client.Tests/SharedSecretProviderTests.cs | Migrates to MSTest and adjusts RSA creation per TFM. |
| test/Microsoft.DotNet.HotReload.Client.Tests/Microsoft.DotNet.HotReload.Client.Tests.csproj | Switches project SDK to MSTest.Sdk, opts out of xUnit defaults, and updates dependencies. |
| test/Microsoft.DotNet.HotReload.Client.Tests/EnvironmentUtilitiesTests.cs | Migrates to MSTest and replaces AssertEx with MSTest assertions. |
| test/Microsoft.DotNet.HotReload.Client.Tests/ArrayBufferWriterTests.cs | Migrates a large test suite to MSTest, including data-driven tests and categories. |
| test/HelixTasks/SDKCustomCreateXUnitWorkItemsWithTestExclusion.cs | Adds MTP-aware Helix command generation (switching away from dotnet test). |
| test/Directory.Build.targets | Avoids applying xUnit-specific defaults/global usings when UseMSTestSdk=true. |
| global.json | Pins MSTest.Sdk version under msbuild-sdks. |
| eng/dependabot/Packages.props | Adds dependabot tracking for Combinatorial.MSTest. |
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 1
…ecks Address PR feedback: replace the Assert.Inconclusive(...) + RuntimeInformation.IsOSPlatform / Environment.Is64BitProcess gates on InvalidAdvance_Large with MSTest declarative ConditionBaseAttributes. - [OSCondition(OperatingSystems.Windows | OperatingSystems.OSX)] replaces the RuntimeInformation.IsOSPlatform check. - New Is64BitProcessConditionAttribute (single-purpose ConditionBaseAttribute) replaces the Environment.Is64BitProcess check. Filed microsoft/testfx#9070 asking testfx to ship a generic ConditionAttribute equivalent of xUnit's [ConditionalFact(typeof(X), nameof(member))] so per-condition shims like this one are no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed PR feedback in commit 8705ccc:
|
|
Replaced the manual Also refreshed the related comments in |
…Sdk)
MSTest.Sdk already sets $(UsingMSTestSdk)=true in its Sdk.props before
Directory.Build.props is evaluated, so the custom <UseMSTestSdk>true</UseMSTestSdk>
opt-in property is redundant. This change:
- Removes <UseMSTestSdk>true</UseMSTestSdk> from MSTest.Sdk csproj(s).
- Renames $(UseMSTestSdk) -> $(UsingMSTestSdk) in test/Directory.Build.targets
(the xUnit-defaults gating condition) and in xunit-runner/{XUnitPublish,XUnitRunner}.targets
(Helix MTP dispatcher detection).
|
Made The new (Applied to all 9 stacked PRs since they share these plumbing files.) |
|
Replaced the custom |
The original xUnit v3 test used TestContext.Current.CancellationToken to propagate
test cancellation into the WriteAsync call. The MSTest migration swapped that for
CancellationToken.None, dropping cancellation propagation.
Restore the original behavior by adding the standard MSTest `public TestContext TestContext { get; set; }`
property on the test class and routing the cancellation token through it.
|
Switched migration-introduced |
|
Round 3: moved The Using is now global for any test project ( Deviation from your literal suggestion: I left the |
--report-trx is an MTP CLI argument provided only when the Microsoft.Testing.Extensions.TrxReport extension is loaded on the test host. MSTest.Sdk's Default/AllMicrosoft profiles enable it by default, but other MTP runners (e.g. xUnit v3 MTP) do not bundle the extension, so passing --report-trx to those hosts fails with 'unknown argument'. XUnitPublish.targets now exposes the GetTrxReportEnabled target which returns the value of EnableMicrosoftTestingExtensionsTrxReport. XUnitRunner.targets calls that target and propagates the value as the EnableTrxReport metadata of SDKCustomXUnitProject items. SDKCustomCreateXUnitWorkItemsWithTestExclusion reads that metadata and only appends --report-trx to the MTP command line when it is true. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…est/Directory.Build.targets Per @Evangelink: keep per-csproj boilerplate minimal. FluentAssertions is now a global using for any test project (gated on IsTestProject OR UsingMSTestSdk), and AwesomeAssertions is added as a PackageReference for MSTest.Sdk projects. xUnit projects continue to pick it up transitively via Microsoft.NET.TestFramework. The Microsoft.NET.TestFramework.* and Xunit usings remain gated on the xUnit branch (UsingMSTestSdk != true) because MSTest projects in this repo do not reference Microsoft.NET.TestFramework; making those usings global would fail with CS0246. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The recent `Helix dispatcher: gate --report-trx on TrxReport extension
being loaded` commit added XML comments in XUnitRunner.targets and
XUnitPublish.targets that included the literal `--report-trx` switch
(and a `--` dash separator). XML 1.0 disallows `--` inside comment
contents, so MSBuild aborts loading test/UnitTests.proj with:
error MSB4024: The imported project file
"test/xunit-runner/XUnitRunner.targets" could not be loaded.
An XML comment cannot contain '--', and '-' cannot be the last
character.
That breaks the Queue Tests stage on every PR that picked up these
files. Rephrase the comments so they no longer contain `--`; the
actual MSBuild logic is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Validation
Q:\src\sdk\.dotnet\dotnet.exe build test\Microsoft.DotNet.HotReload.Client.Tests\Microsoft.DotNet.HotReload.Client.Tests.csproj --nologo— 0 warnings, 0 errors.Q:\src\sdk\.dotnet\dotnet.exe exec artifacts\bin\Microsoft.DotNet.HotReload.Client.Tests\Debug\net11.0\Microsoft.DotNet.HotReload.Client.Tests.dll— 81 passed.artifacts\bin\Microsoft.DotNet.HotReload.Client.Tests\Debug\net472\Microsoft.DotNet.HotReload.Client.Tests.exe— 81 passed.