MSTest.Sdk: do not use IsImplictlyDefined (#2880)

microsoft · May 21, 2024 · f77b630 · f77b630
1 parent a0515c5
commit f77b630
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 25 deletions.
diff --git a/src/Package/MSTest.Sdk/README.md b/src/Package/MSTest.Sdk/README.md
@@ -0,0 +1,15 @@
+# MSTest SDK
+
+## Design notes
+
+Do not use the `IsImplictlyDefined="true"` attribute in the `PackageReference` element in the `.targets` files. Instead,
+rely on the `VersionOverride` attribute to define the package version. This is because for big projects, teams are usually
+slowly migrating to MSTest.Sdk so they need to keep defining MSTest (and platform) packages in their CPM file.
+
+If we use 'IsImplicitlyDefined' attribute, the package will be "defined twice" which will lead to `NU1009` warnings that
+are most of the time updated as errors (warnAsError). We created a thread with MSBuild and NuGet teams and the only suggested
+solution is for users to suppress the warning which is not ideal. Until a better solution is provided, we will use the
+`VersionOverride` trick instead as it achieves a relatively close behavior while preventing the warning.
+
+We could also consider having a property like `MSTestSdkDisableIsImplicitlyDefinedAttribute` that users can set to `true` to
+disable the `IsImplicitlyDefined` attribute in the `.targets` files but we don't see any strong reason to do that at the moment.
diff --git a/src/Package/MSTest.Sdk/Sdk/Features/Aspire.targets b/src/Package/MSTest.Sdk/Sdk/Features/Aspire.targets
@@ -2,7 +2,7 @@
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
  <ItemGroup>
- <PackageReference Include="Aspire.Hosting.Testing" Version="$(AspireHostingTestingVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
+ <PackageReference Include="Aspire.Hosting.Testing" Version="$(AspireHostingTestingVersion)" VersionOverride="$(AspireHostingTestingVersion)" Sdk="MSTest" />
  </ItemGroup>
 
  <!--

diff --git a/src/Package/MSTest.Sdk/Sdk/Features/Playwright.targets b/src/Package/MSTest.Sdk/Sdk/Features/Playwright.targets
@@ -2,7 +2,7 @@
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
  <ItemGroup>
- <PackageReference Include="Microsoft.Playwright.MSTest" Version="$(MicrosoftPlaywrightVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Playwright.MSTest" Version="$(MicrosoftPlaywrightVersion)" VersionOverride="$(MicrosoftPlaywrightVersion)" Sdk="MSTest" />
  </ItemGroup>
 
  <!--

diff --git a/src/Package/MSTest.Sdk/Sdk/Runner/ClassicEngine.targets b/src/Package/MSTest.Sdk/Sdk/Runner/ClassicEngine.targets
@@ -28,20 +28,20 @@
 
  <!-- Core -->
  <ItemGroup>
- <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.Analyzers" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMSTestAnalyzers)' != 'false' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkVersion)" VersionOverride="$(MicrosoftNETTestSdkVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.Analyzers" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Condition=" '$(EnableMSTestAnalyzers)' != 'false' " Sdk="MSTest" />
  </ItemGroup>
 
  <!-- Extensions -->
  <ItemGroup>
- <PackageReference Include="Microsoft.Testing.Extensions.TrxReport" Version="$(MicrosoftTestingExtensionsTrxReportVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsTrxReport)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.CrashDump" Version="$(MicrosoftTestingExtensionsCrashDumpVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsCrashDump)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.HangDump" Version="$(MicrosoftTestingExtensionsHangDumpVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsHangDump)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.CodeCoverage" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.HotReload" Version="$(MicrosoftTestingExtensionsHotReloadVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsHotReload)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.Retry" Version="$(MicrosoftTestingExtensionsRetryVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsRetry)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.TrxReport" Version="$(MicrosoftTestingExtensionsTrxReportVersion)" VersionOverride="$(MicrosoftTestingExtensionsTrxReportVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsTrxReport)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.CrashDump" Version="$(MicrosoftTestingExtensionsCrashDumpVersion)" VersionOverride="$(MicrosoftTestingExtensionsCrashDumpVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsCrashDump)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.HangDump" Version="$(MicrosoftTestingExtensionsHangDumpVersion)" VersionOverride="$(MicrosoftTestingExtensionsHangDumpVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsHangDump)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.CodeCoverage" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" VersionOverride="$(MicrosoftTestingExtensionsCodeCoverageVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.HotReload" Version="$(MicrosoftTestingExtensionsHotReloadVersion)" VersionOverride="$(MicrosoftTestingExtensionsHotReloadVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsHotReload)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.Retry" Version="$(MicrosoftTestingExtensionsRetryVersion)" VersionOverride="$(MicrosoftTestingExtensionsRetryVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsRetry)' == 'true' " Sdk="MSTest" />
  </ItemGroup>
 
  <Import Project="$(MSBuildThisFileDirectory)../Features/Aspire.targets" Condition=" '$(EnableAspireTesting)' == 'true' " />

diff --git a/src/Package/MSTest.Sdk/Sdk/Runner/NativeAOT.targets b/src/Package/MSTest.Sdk/Sdk/Runner/NativeAOT.targets
@@ -13,18 +13,18 @@
 
  <!-- Core -->
  <ItemGroup>
- <PackageReference Include="Microsoft.Testing.Platform.MSBuild" Version="$(MicrosoftTestingPlatformVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.Engine" Version="$(MSTestEngineVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.SourceGeneration" Version="$(MSTestEngineVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Platform.MSBuild" Version="$(MicrosoftTestingPlatformVersion)" VersionOverride="$(MicrosoftTestingPlatformVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.Engine" Version="$(MSTestEngineVersion)" VersionOverride="$(MSTestEngineVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.SourceGeneration" Version="$(MSTestEngineVersion)" VersionOverride="$(MSTestEngineVersion)" Sdk="MSTest" />
  </ItemGroup>
 
  <!-- Extensions -->
  <ItemGroup>
- <PackageReference Include="Microsoft.Testing.Extensions.TrxReport" Version="$(MicrosoftTestingExtensionsTrxReportVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsTrxReport)' == 'true' " Sdk="MSTest" />
- <PackageReference Include="Microsoft.Testing.Extensions.CodeCoverage" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.TrxReport" Version="$(MicrosoftTestingExtensionsTrxReportVersion)" VersionOverride="$(MicrosoftTestingExtensionsTrxReportVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsTrxReport)' == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.Testing.Extensions.CodeCoverage" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" VersionOverride="$(MicrosoftTestingExtensionsCodeCoverageVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' " Sdk="MSTest" />
  <!-- Support for -p:AotMsCodeCoverageInstrumentation="true" during dotnet publish for native aot -->
- <PackageReference Include="Microsoft.CodeCoverage.MSBuild" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' and $(PublishAot) == 'true' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.CodeCoverage.MSBuild" Version="$(MicrosoftTestingExtensionsCodeCoverageVersion)" VersionOverride="$(MicrosoftTestingExtensionsCodeCoverageVersion)" Condition=" '$(EnableMicrosoftTestingExtensionsCodeCoverage)' == 'true' and $(PublishAot) == 'true' " Sdk="MSTest" />
  </ItemGroup>
 
 </Project>
diff --git a/src/Package/MSTest.Sdk/Sdk/VSTest/VSTest.targets b/src/Package/MSTest.Sdk/Sdk/VSTest/VSTest.targets
@@ -2,10 +2,10 @@
 <Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
  <ItemGroup>
- <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Sdk="MSTest" />
- <PackageReference Include="MSTest.Analyzers" Version="$(MSTestVersion)" IsImplicitlyDefined="True" Condition=" '$(EnableMSTestAnalyzers)' != 'false' " Sdk="MSTest" />
+ <PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNETTestSdkVersion)" VersionOverride="$(MicrosoftNETTestSdkVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.TestAdapter" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.TestFramework" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Sdk="MSTest" />
+ <PackageReference Include="MSTest.Analyzers" Version="$(MSTestVersion)" VersionOverride="$(MSTestVersion)" Condition=" '$(EnableMSTestAnalyzers)' != 'false' " Sdk="MSTest" />
  </ItemGroup>
 
  <!--

diff --git a/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/SdkTests.cs b/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/SdkTests.cs
@@ -187,7 +187,7 @@ public async Task RunTests_With_CentralPackageManagement_Standalone(string multi
 </Project>
 """);
 
- DotnetMuxerResult compilationResult = await DotnetCli.RunAsync($"build -c {buildConfiguration} {generator.TargetAssetPath}", _acceptanceFixture.NuGetGlobalPackagesFolder.Path);
+ DotnetMuxerResult compilationResult = await DotnetCli.RunAsync($"build -c {buildConfiguration} {generator.TargetAssetPath} /warnAsError", _acceptanceFixture.NuGetGlobalPackagesFolder.Path);
  Assert.AreEqual(0, compilationResult.ExitCode);
  foreach (string tfm in multiTfm.Split(";"))
  {
@@ -264,7 +264,7 @@ public async Task RunTests_With_MSTestRunner_Standalone_Selectively_Enabled_Exte
 </Project>
 """);
 
- DotnetMuxerResult compilationResult = await DotnetCli.RunAsync($"build -c {buildConfiguration} {generator.TargetAssetPath}", _acceptanceFixture.NuGetGlobalPackagesFolder.Path);
+ DotnetMuxerResult compilationResult = await DotnetCli.RunAsync($"build -c {buildConfiguration} {generator.TargetAssetPath} /warnAsError", _acceptanceFixture.NuGetGlobalPackagesFolder.Path);
  Assert.AreEqual(0, compilationResult.ExitCode);
  foreach (string tfm in multiTfm.Split(";"))
  {