From dec5515f3635e390a5e9353347a847d3a5500d5d Mon Sep 17 00:00:00 2001
From: Viktor Hofer <viktor.hofer@microsoft.com>
Date: Tue, 21 May 2024 13:25:34 +0000
Subject: [PATCH] Simplify scenario-tests test invocation & define dependencies

1. Simplify scenario-tests test invocation by avoiding
   the intermediate NoTargets wrapper project.
2. Declare the dependency on extract-sdk-archive.proj
   in the test projects directly to avoid race
   conditions. https://github.com/dotnet/sdk/pull/41014
   failed because the scenario-tests already ran
   but the SDK test folder wasn't yet ready.
---
 .../repo-projects/Directory.Build.targets     | 29 +++++++++++++
 .../content/repo-projects/scenario-tests.proj | 42 ++++++++-----------
 ...osoft.DotNet.SourceBuild.SmokeTests.csproj |  3 +-
 ...Microsoft.DotNet.UnifiedBuild.Tests.csproj |  3 +-
 .../scenario-tests/Directory.Build.targets    |  9 ----
 .../test/scenario-tests/scenario-tests.proj   | 17 --------
 src/SourceBuild/content/test/tests.proj       |  9 ++--
 7 files changed, 57 insertions(+), 55 deletions(-)
 delete mode 100644 src/SourceBuild/content/test/scenario-tests/Directory.Build.targets
 delete mode 100644 src/SourceBuild/content/test/scenario-tests/scenario-tests.proj

diff --git a/src/SourceBuild/content/repo-projects/Directory.Build.targets b/src/SourceBuild/content/repo-projects/Directory.Build.targets
index ee1c94163b80..b6265ee9df7d 100644
--- a/src/SourceBuild/content/repo-projects/Directory.Build.targets
+++ b/src/SourceBuild/content/repo-projects/Directory.Build.targets
@@ -620,6 +620,35 @@
     ExtractToolPackage;
     CleanupRepo" />
 
+  <Target Name="Test"
+          Inputs="$(MSBuildProjectFullPath)"
+          Outputs="$(BaseIntermediateOutputPath)Test.complete">
+    <Message Importance="High" Text="[$([System.DateTime]::Now.ToString('HH:mm:ss.ff'))] Testing $(RepositoryName)" />
+    <Message Importance="High" Text="Running command:" />
+    <Message Importance="High" Text="  $(TestCommand)" />
+    <Message Importance="High" Text="  With Environment Variables:"/>
+    <Message Importance="High" Text="    %(TestEnvironmentVariable.Identity)" />
+
+    <PropertyGroup>
+      <FullTestCommand>$(TestCommand)</FullTestCommand>
+      <FullTestCommand Condition="'$(LogVerbosityOptOut)' != 'true'">$(FullTestCommand) /v:$(LogVerbosity)</FullTestCommand>
+    </PropertyGroup>
+
+    <Exec Command="$(FullTestCommand)"
+          WorkingDirectory="$(ProjectDirectory)"
+          EnvironmentVariables="@(TestEnvironmentVariable)">
+      <Output TaskParameter="ExitCode" PropertyName="RepoTestExitCode" />
+    </Exec>
+
+    <Message Importance="High" Text="[$([System.DateTime]::Now.ToString('HH:mm:ss.ff'))] Testing $(RepositoryName)...done" />
+
+    <MakeDir Directories="$(BaseIntermediateOutputPath)" />
+    <Touch Files="$(BaseIntermediateOutputPath)Test.complete" AlwaysCreate="true">
+      <Output TaskParameter="TouchedFiles" ItemName="FileWrites" />
+    </Touch>
+  </Target>
+  <Target Name="VSTest" DependsOnTargets="Test" />
+
   <UsingTask TaskName="Microsoft.DotNet.UnifiedBuild.Tasks.UsageReport.WritePackageUsageData" AssemblyFile="$(MicrosoftDotNetUnifiedBuildTasksAssembly)" TaskFactory="TaskHostFactory" />
   <Target Name="WritePrebuiltUsageData"
           Inputs="$(MSBuildProjectFullPath)"
diff --git a/src/SourceBuild/content/repo-projects/scenario-tests.proj b/src/SourceBuild/content/repo-projects/scenario-tests.proj
index 2462e75b64e7..0cca99551b1f 100644
--- a/src/SourceBuild/content/repo-projects/scenario-tests.proj
+++ b/src/SourceBuild/content/repo-projects/scenario-tests.proj
@@ -1,5 +1,15 @@
 <Project Sdk="Microsoft.Build.NoTargets" TreatAsLocalProperty="CleanWhileBuilding">
 
+  <PropertyGroup>
+    <!-- The scenario-tests repo shouldn't be cleaned after building as we run tests from it. -->
+    <CleanWhileBuilding>false</CleanWhileBuilding>
+
+    <ScenarioTestsArtifactsDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'scenario-tests'))</ScenarioTestsArtifactsDir>
+    <ScenarioTestsResultsDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsTestResultsDir)', 'scenario-tests'))</ScenarioTestsResultsDir>
+    <NuGetConfigInputForScenarioTests>$([MSBuild]::NormalizePath('$(SrcDir)', 'sdk', 'NuGet.config'))</NuGetConfigInputForScenarioTests>
+    <NuGetConfigOutputForScenarioTests>$(ScenarioTestsArtifactsDir)NuGet.config</NuGetConfigOutputForScenarioTests>
+  </PropertyGroup>
+
   <ItemGroup>
     <RepositoryReference Include="arcade" />
     <RepositoryReference Include="command-line-api" />
@@ -10,28 +20,16 @@
     <RepositoryReference Include="source-build-reference-packages" />
   </ItemGroup>
 
-  <PropertyGroup>
-    <!-- The scenario-tests repo shouldn't be cleaned after building as we run tests from it. -->
-    <CleanWhileBuilding>false</CleanWhileBuilding>
-
-    <ScenarioTestsArtifactsDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsDir)', 'scenario-tests'))</ScenarioTestsArtifactsDir>
-    <ScenarioTestsResultsDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsTestResultsDir)', 'scenario-tests'))</ScenarioTestsResultsDir>
-    <_ScenarioTestsNuGetConfig>$(ScenarioTestsArtifactsDir)NuGet.config</_ScenarioTestsNuGetConfig>
-    <_SdkNuGetConfig>$([MSBuild]::NormalizePath('$(SrcDir)', 'sdk', 'NuGet.config'))</_SdkNuGetConfig>
-  </PropertyGroup>
-
   <Target Name="SetupNuGetConfig"
-          Inputs="$(_SdkNuGetConfig)"
-          Outputs="$(_ScenarioTestsNuGetConfig)">
-
-    <Copy SourceFiles="$(_SdkNuGetConfig)"
-          DestinationFiles="$(_ScenarioTestsNuGetConfig)" />
-
+          Inputs="$(NuGetConfigInputForScenarioTests)"
+          Outputs="$(NuGetConfigOutputForScenarioTests)">
+    <Copy SourceFiles="$(NuGetConfigInputForScenarioTests)"
+          DestinationFiles="$(NuGetConfigOutputForScenarioTests)" />
   </Target>
 
-  <Target Name="RunScenarioTests"
-          DependsOnTargets="SetupNuGetConfig;DetermineSourceBuiltSdkVersion">
-    
+  <Target Name="PrepareScenarioTestsInputs"
+          DependsOnTargets="SetupNuGetConfig;DetermineSourceBuiltSdkVersion"
+          BeforeTargets="Test">
     <PropertyGroup>
       <_CurrentDateTime>$([System.DateTime]::Now.ToString("yyyy-MM-dd_HH_mm_ss"))</_CurrentDateTime>
       <_TestXmlOutputPath>$(ScenarioTestsResultsDir)$(_CurrentDateTime).xml</_TestXmlOutputPath>
@@ -49,7 +47,7 @@
     <MakeDir Directories="$(ScenarioTestsResultsDir)" />
 
     <ItemGroup>
-      <_ScenarioTestEnvVars Include="
+      <TestEnvironmentVariable Include="
         TestRoot=$(_TestRoot);
         DotNetRoot=$(DotNetSdkExtractDir);
         TestSdkVersion=$(SourceBuiltSdkVersion);
@@ -57,10 +55,6 @@
         DotNetTool=$(_DotNetTool);
         _InitializeDotNetCli=$(DotNetSdkExtractDir)" />
     </ItemGroup>
-
-    <Exec Command="$(TestCommand)"
-          WorkingDirectory="$(ProjectDirectory)"
-          EnvironmentVariables="@(_ScenarioTestEnvVars)" />
   </Target>
 
 </Project>
diff --git a/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj b/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj
index 0448a8064331..e298aca90d7f 100644
--- a/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj
+++ b/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj
@@ -17,6 +17,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <ProjectReference Include="$(RepositoryEngineeringDir)extract-sdk-archive.proj" ReferenceOutputAssembly="false" />
     <ProjectReference Include="..\TestUtilities\TestUtilities.csproj" />
   </ItemGroup>
 
@@ -71,7 +72,7 @@
       </RuntimeHostConfigurationOption>
       <RuntimeHostConfigurationOption Include="$(MSBuildProjectName).MsftSdkTarballPath">
         <Value>$(MsftSdkTarballPath)</Value>
-      </RuntimeHostConfigurationOption>      
+      </RuntimeHostConfigurationOption>
       <RuntimeHostConfigurationOption Include="$(MSBuildProjectName).PoisonReportPath"
                                       Condition="'$(EnablePoison)' == 'true'">
         <Value>$(PoisonUsageReportFile)</Value>
diff --git a/src/SourceBuild/content/test/Microsoft.DotNet.UnifiedBuild.Tests/Microsoft.DotNet.UnifiedBuild.Tests.csproj b/src/SourceBuild/content/test/Microsoft.DotNet.UnifiedBuild.Tests/Microsoft.DotNet.UnifiedBuild.Tests.csproj
index 86b3f1f15955..3a2bdeeb632f 100644
--- a/src/SourceBuild/content/test/Microsoft.DotNet.UnifiedBuild.Tests/Microsoft.DotNet.UnifiedBuild.Tests.csproj
+++ b/src/SourceBuild/content/test/Microsoft.DotNet.UnifiedBuild.Tests/Microsoft.DotNet.UnifiedBuild.Tests.csproj
@@ -11,7 +11,8 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" />
     <PackageReference Include="NuGet.Protocol" />
-    <ProjectReference Include="$(RepositoryEngineeringDir)merge-asset-manifests.proj" ReferenceOutputAssembly="false" PrivateAssets="all" />
+    <ProjectReference Include="$(RepositoryEngineeringDir)extract-sdk-archive.proj" ReferenceOutputAssembly="false" />
+    <ProjectReference Include="$(RepositoryEngineeringDir)merge-asset-manifests.proj" ReferenceOutputAssembly="false" />
   </ItemGroup>
 
   <ItemGroup>
diff --git a/src/SourceBuild/content/test/scenario-tests/Directory.Build.targets b/src/SourceBuild/content/test/scenario-tests/Directory.Build.targets
deleted file mode 100644
index 56dccb4796da..000000000000
--- a/src/SourceBuild/content/test/scenario-tests/Directory.Build.targets
+++ /dev/null
@@ -1,9 +0,0 @@
-<Project>
-
-  <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.targets, $(MSBuildThisFileDirectory)..))" />
-
-  <!-- scenario-tests test execution requires a custom test target. -->
-  <Target Name="Test" DependsOnTargets="RunScenarioTests" />
-  <Target Name="VSTest" DependsOnTargets="RunScenarioTests" />
-
-</Project>
diff --git a/src/SourceBuild/content/test/scenario-tests/scenario-tests.proj b/src/SourceBuild/content/test/scenario-tests/scenario-tests.proj
deleted file mode 100644
index 410ac2f54c4c..000000000000
--- a/src/SourceBuild/content/test/scenario-tests/scenario-tests.proj
+++ /dev/null
@@ -1,17 +0,0 @@
-<Project Sdk="Microsoft.Build.NoTargets">
-
-  <PropertyGroup>
-    <TargetFramework>$(NetCurrent)</TargetFramework>
-    <IsTestProject>true</IsTestProject>
-  </PropertyGroup>
-
-  <!-- Scenarios tests are handled separately from the other test projects. This is because the output of the scenario tests is
-       an executable that is used to run the tests and so using the VSTest runner doesn't work with them. They also require
-       some setup steps to be configured with the VMR. -->
-  <Target Name="RunScenarioTests">
-    <MSBuild Projects="$(RepoProjectsDir)scenario-tests.proj"
-             Properties="Configuration=$(Configuration)"
-             Targets="RunScenarioTests" />
-  </Target>
-
-</Project>
diff --git a/src/SourceBuild/content/test/tests.proj b/src/SourceBuild/content/test/tests.proj
index 2f8df9ccb659..83314d1fa783 100644
--- a/src/SourceBuild/content/test/tests.proj
+++ b/src/SourceBuild/content/test/tests.proj
@@ -4,7 +4,7 @@
     <_RunScenarioTests>true</_RunScenarioTests>
 
     <!-- Skip scenario tests if the host architecture is different from the target architecture since the tests
-        require the ability to execute the built SDK. But the CLI is not capable of running on a host with a 
+        require the ability to execute the built SDK. But the CLI is not capable of running on a host with a
         different architecture (i.e. "cannot execute binary file: Exec format error"). -->
     <_RunScenarioTests Condition="'$(BuildArchitecture.ToLowerInvariant())' != '$(TargetArchitecture.ToLowerInvariant())'">false</_RunScenarioTests>
 
@@ -18,12 +18,15 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <ProjectReference Include="$(RepositoryEngineeringDir)extract-sdk-archive.proj" />
     <ProjectReference Include="Microsoft.DotNet.SourceBuild.SmokeTests\Microsoft.DotNet.SourceBuild.SmokeTests.csproj"
                       Condition="'$(DotNetBuildSourceOnly)' == 'true'" />
     <ProjectReference Include="Microsoft.DotNet.UnifiedBuild.Tests\Microsoft.DotNet.UnifiedBuild.Tests.csproj"
                       Condition="'$(ShortStack)' != 'true' and '$(PortableBuild)' == 'true' and '$(PgoInstrument)' != 'true'" />
-    <ProjectReference Include="scenario-tests\scenario-tests.proj" Condition="'$(_RunScenarioTests)' == 'true'" />
+
+    <!-- Make sure that the sdk archive is extracted before running the scenario-tests. Need to do this here as it's hard
+         to define the dependency directly in scneario-tests.proj. -->
+    <ProjectReference Include="$(RepositoryEngineeringDir)extract-sdk-archive.proj" BuildInParallel="false" />
+    <ProjectReference Include="$(RepoProjectsDir)scenario-tests.proj" Condition="'$(_RunScenarioTests)' == 'true'" BuildInParallel="false" />
   </ItemGroup>
 
 </Project>