From 230e4a02efd0957f3a7af9e45d15374e440ee661 Mon Sep 17 00:00:00 2001
From: Nate McMaster <natemcmaster@users.noreply.github.com>
Date: Fri, 12 Jan 2018 09:46:36 -0800
Subject: [PATCH] Split compilation and tests into separate phases (#779)

* Split compilation and tests into separate phases
* Ensure template tests are skipped, and reduce duplication between test/build repo targets
* Show summary at the end of which repos failed
---
 build/RepositoryBuild.targets | 108 ++++++++++++++++++++++++++--------
 build/Templating.targets      |   9 +--
 build/repo.targets            |   8 +--
 build/tasks/OrderBy.cs        |  28 +++++++++
 build/tasks/RepoTasks.tasks   |   1 +
 5 files changed, 119 insertions(+), 35 deletions(-)
 create mode 100644 build/tasks/OrderBy.cs

diff --git a/build/RepositoryBuild.targets b/build/RepositoryBuild.targets
index 0e0216fbde..7a045b06cf 100644
--- a/build/RepositoryBuild.targets
+++ b/build/RepositoryBuild.targets
@@ -1,5 +1,15 @@
 <Project>
-  <Target Name="_BuildRepositories" DependsOnTargets="GeneratePropsFiles">
+
+  <PropertyGroup>
+    <!-- Experimental flag to run assemblies AND repos tests in parallel...if you dare. -->
+    <TestReposInParallel>false</TestReposInParallel>
+
+    <_NoBuildRepos>$(NoBuild)</_NoBuildRepos>
+    <_BuildScriptToExecute Condition="'$(OS)'!='Windows_NT'">build.sh</_BuildScriptToExecute>
+    <_BuildScriptToExecute Condition="'$(OS)'=='Windows_NT'">build.cmd</_BuildScriptToExecute>
+  </PropertyGroup>
+
+  <Target Name="GetRepoBatches" DependsOnTargets="GeneratePropsFiles;ComputeGraph">
     <ItemGroup>
       <BatchedRepository Include="$(MSBuildProjectFullPath)">
         <BuildGroup>%(RepositoryBuildOrder.Order)</BuildGroup>
@@ -10,33 +20,52 @@
         </AdditionalProperties>
       </BatchedRepository>
     </ItemGroup>
+  </Target>
 
-    <PropertyGroup>
-      <BatchBuilds Condition="$(BuildInParallel) AND '$(SkipTests)'=='true'">true</BatchBuilds>
-      <BatchBuilds Condition="'$(BatchBuilds)'==''">false</BatchBuilds>
-    </PropertyGroup>
-
-    <ItemGroup>
-      <PinnedArtifactInfo Include="@(ArtifactInfo)" Condition="'$(BuildGraphOf)' == ''" />
-    </ItemGroup>
-
-    <!-- If we are building a sub-graph, we need to re-examine the list of artifacts that will actual build. -->
-    <MSBuild Projects="$(MSBuildProjectFullPath)"
-             Targets="GetArtifactInfo"
-             Properties="RepositoryRoot=$(SubmoduleRoot)%(RepositoryBuildOrder.Identity)\;Configuration=$(Configuration);BuildNumber=$(BuildNumber);DesignTimeBuild=true"
-             Condition="'$(BuildGraphOf)' != ''">
-      <Output TaskParameter="TargetOutputs" ItemName="PinnedArtifactInfo" />
-    </MSBuild>
-
+  <Target Name="_BuildRepositories" DependsOnTargets="GetRepoBatches">
     <MSBuild
       Projects="@(BatchedRepository)"
-      BuildInParallel="$(BatchBuilds)"
+      BuildInParallel="true"
       StopOnFirstFailure="true"
       Targets="_BuildRepository"
       Properties="BuildGroup=%(BatchedRepository.BuildGroup);BuildNumber=$(BuildNumber);IsFinalBuild=$(IsFinalBuild);Configuration=$(Configuration)" />
+
+    <PropertyGroup>
+      <_NoBuildRepos>true</_NoBuildRepos>
+    </PropertyGroup>
   </Target>
 
-  <Target Name="_BuildRepository">
+  <Target Name="_TestRepositories" DependsOnTargets="GetRepoBatches">
+    <!--
+      Use the task to sort instead of batching (i.e. using %(BatchedRepository.BuildGroup))
+      When batching, StopOnFirstFailure doesn't help because the MSBuild task would be invoked multiple times
+      instead of invoking once with many projects.
+    -->
+    <RepoTasks.OrderBy Items="@(BatchedRepository)" Key="BuildGroup">
+      <Output TaskParameter="Items" ItemName="_BatchedTestRepo" />
+    </RepoTasks.OrderBy>
+
+    <MSBuild
+      Projects="@(_BatchedTestRepo)"
+      BuildInParallel="$(TestProjectsInParallel)"
+      StopOnFirstFailure="false"
+      Targets="_TestRepository"
+      Properties="BuildNumber=$(BuildNumber);IsFinalBuild=$(IsFinalBuild);Configuration=$(Configuration);_NoBuildRepos=$(_NoBuildRepos)"
+      ContinueOnError="true">
+      <Output TaskParameter="TargetOutputs" ItemName="_RepoTestResults" />
+    </MSBuild>
+
+    <Warning Text="No test results were found from running repos." Condition="@(_RepoTestResults->Count()) == 0"/>
+    <Message Text="Tests passed for the following repos:%0A - @(_RepoTestResults->WithMetadataValue('Success', 'true'), '%0A - ')"
+      Importance="High"
+      Condition="@(_RepoTestResults->WithMetadataValue('Success', 'true')->Count()) != 0 " />
+    <Error Text="Tests failed for the following repos:%0A - @(_RepoTestResults->WithMetadataValue('Success', 'false'), '%0A - ')"
+      Condition="@(_RepoTestResults->WithMetadataValue('Success', 'false')->Count()) != 0 " />
+  </Target>
+
+  <!-- Inner build context -->
+
+  <Target Name="GetRepoBuildProps">
     <PropertyGroup>
       <!-- Should reduce allowable package feeds to only nuget.org. -->
       <RepositoryBuildArguments>$(RepositoryBuildArguments) /p:AspNetUniverseBuildOffline=true</RepositoryBuildArguments>
@@ -47,14 +76,15 @@
       <RepositoryBuildArguments>$(RepositoryBuildArguments) /p:Configuration=$(Configuration)</RepositoryBuildArguments>
       <RepositoryBuildArguments>$(RepositoryBuildArguments) /p:IsFinalBuild=$(IsFinalBuild)</RepositoryBuildArguments>
       <RepositoryBuildArguments>$(RepositoryBuildArguments) /noconsolelogger '/l:RepoTasks.FlowLogger,$(MSBuildThisFileDirectory)tasks\bin\publish\RepoTasks.dll;Summary;FlowId=$(RepositoryToBuild)'</RepositoryBuildArguments>
+    </PropertyGroup>
+  </Target>
 
-      <BuildArguments>/t:CleanArtifacts $(_RepositoryBuildTargets) $(RepositoryBuildArguments)</BuildArguments>
+  <Target Name="_BuildRepository" DependsOnTargets="GetRepoBuildProps">
+    <PropertyGroup>
+      <BuildArguments>/t:CleanArtifacts /t:Build /p:SkipTests=true $(RepositoryBuildArguments)</BuildArguments>
       <RepositoryArtifactsRoot>$(BuildRepositoryRoot)artifacts</RepositoryArtifactsRoot>
       <RepositoryArtifactsBuildDirectory>$(RepositoryArtifactsRoot)\build\</RepositoryArtifactsBuildDirectory>
       <RepositoryArtifactsMSBuildDirectory>$(RepositoryArtifactsRoot)\msbuild\</RepositoryArtifactsMSBuildDirectory>
-
-      <_BuildScriptToExecute Condition="'$(OS)'!='Windows_NT'">build.sh</_BuildScriptToExecute>
-      <_BuildScriptToExecute Condition="'$(OS)'=='Windows_NT'">build.cmd</_BuildScriptToExecute>
     </PropertyGroup>
 
     <Message Text="============ Building $(RepositoryToBuild) ============" Importance="High" />
@@ -84,4 +114,34 @@
     <Message Text="============ Done building $(RepositoryToBuild) ============" Importance="High" />
   </Target>
 
+  <Target Name="_TestRepository" DependsOnTargets="GetRepoBuildProps" Returns="@(RepositoryTestResult)">
+    <PropertyGroup>
+      <BuildArguments>/t:Test /p:NoBuild=$(_NoBuildRepos) $(RepositoryBuildArguments)</BuildArguments>
+    </PropertyGroup>
+
+    <ItemGroup>
+      <RepositoryTestResult Include="$(RepositoryToBuild)" Success="false" />
+    </ItemGroup>
+
+    <Message Text="============ Testing $(RepositoryToBuild) ============" Importance="High" />
+
+    <!-- Copy Korebuild lock file to individual repos to align version if the repo doesn't already have one -->
+    <Message Text="Copying KoreBuild lockfile from Universe to repository $(BuildRepositoryRoot)"/>
+    <Copy SourceFiles="$(RepositoryRoot)korebuild-lock.txt" DestinationFolder="$(BuildRepositoryRoot)" />
+
+    <Exec
+      Command="./$(_BuildScriptToExecute) -Path $(BuildRepositoryRoot) $(BuildArguments)"
+      IgnoreStandardErrorWarningFormat="true"
+      WorkingDirectory="$(RepositoryRoot)"
+      IgnoreExitCode="true">
+      <Output TaskParameter="ExitCode" PropertyName="TestExitCode" />
+    </Exec>
+
+    <ItemGroup>
+      <RepositoryTestResult Update="$(RepositoryToBuild)" Success="true" Condition="'$(TestExitCode)' == '0'" />
+    </ItemGroup>
+
+    <Message Text="============ Done testing $(RepositoryToBuild) ============" Importance="High" />
+  </Target>
+
 </Project>
diff --git a/build/Templating.targets b/build/Templating.targets
index 09300567f0..c199b1c775 100644
--- a/build/Templating.targets
+++ b/build/Templating.targets
@@ -14,16 +14,11 @@
         IsFinalBuild=$(IsFinalBuild);
         SkipAspNetCoreRuntimeInstall=true;
       </TemplateProjProperties>
-      <TemplateProjTargets>CleanArtifacts;Restore;Compile;Package</TemplateProjTargets>
-      <TemplateProjTargets Condition="'$(SkipTests)' != 'true'">
-        $(TemplateProjTargets);
-        Test;
-      </TemplateProjTargets>
     </PropertyGroup>
 
     <MSBuild Projects="$(MSBuildProjectFullPath)"
-             Targets="$(TemplateProjTargets)"
-             Properties="$(TemplateProjProperties)" />
+             Targets="CleanArtifacts;Build"
+             Properties="$(TemplateProjProperties);SkipTests=true" />
 
     <ItemGroup>
       <TemplateArtifacts Include="$(TemplatingProjectRoot)artifacts\build\*" />
diff --git a/build/repo.targets b/build/repo.targets
index d1477e8be7..a9b2998960 100644
--- a/build/repo.targets
+++ b/build/repo.targets
@@ -8,20 +8,19 @@
   <Import Project="Publish.targets" />
 
   <PropertyGroup>
-    <_RepositoryBuildTargets Condition="'$(_RepositoryBuildTargets)'=='' AND '$(SkipTests)'=='true'">/t:Package /t:VerifyPackages</_RepositoryBuildTargets>
-    <_RepositoryBuildTargets Condition="'$(_RepositoryBuildTargets)'==''">/t:Build</_RepositoryBuildTargets>
     <!-- For external packages that come from feeds will mirrored to aspnetcore feeds. -->
     <IntermediateMirrorPackageDir>$(IntermediateDir)mirror\</IntermediateMirrorPackageDir>
     <!-- For external packages that come from feeds we don't mirror. -->
     <IntermediateExternalPackageDir>$(IntermediateDir)ext\</IntermediateExternalPackageDir>
-    <GeneratedPackageVersionPropsPath>$(IntermediateDir)dependencies.props</GeneratedPackageVersionPropsPath>
-    <GeneratedRestoreSourcesPropsPath>$(IntermediateDir)sources.props</GeneratedRestoreSourcesPropsPath>
+    <GeneratedPackageVersionPropsPath>$(IntermediateDir)dependencies.g.props</GeneratedPackageVersionPropsPath>
+    <GeneratedRestoreSourcesPropsPath>$(IntermediateDir)sources.g.props</GeneratedRestoreSourcesPropsPath>
 
     <PrepareDependsOn>$(PrepareDependsOn);VerifyPackageArtifactConfig;PrepareOutputPaths</PrepareDependsOn>
     <CleanDependsOn>$(CleanDependsOn);CleanArtifacts;CleanUniverseArtifacts</CleanDependsOn>
     <RestoreDependsOn>$(RestoreDependsOn);InstallDotNet;RestoreExternalDependencies</RestoreDependsOn>
     <CompileDependsOn>$(CompileDependsOn);BuildRepositories</CompileDependsOn>
     <PackageDependsOn Condition="'$(TestOnly)' != 'true'">$(PackageDependsOn);BuildMetapackage;BuildTemplates;SplitPackages</PackageDependsOn>
+    <TestDependsOn>$(TestDependsOn);_TestRepositories</TestDependsOn>
     <VerifyDependsOn Condition="'$(TestOnly)' != 'true'">$(VerifyDependsOn);VerifyCoherentVersions</VerifyDependsOn>
     <GetArtifactInfoDependsOn>$(GetArtifactInfoDependsOn);ResolveRepoInfo</GetArtifactInfoDependsOn>
   </PropertyGroup>
@@ -114,6 +113,7 @@
 
     <PackNuSpec NuSpecPath="$(MSBuildThisFileDirectory)lineups\Internal.AspNetCore.Universe.Lineup.nuspec"
                 DestinationFolder="$(LineupBuildDir)"
+                Overwrite="true"
                 Properties="version=$(Version);dependenciesPropsFile=$(GeneratedPackageVersionPropsPath)">
       <Output TaskParameter="Packages" ItemName="LineupPackage" />
     </PackNuSpec>
diff --git a/build/tasks/OrderBy.cs b/build/tasks/OrderBy.cs
new file mode 100644
index 0000000000..a761d6f093
--- /dev/null
+++ b/build/tasks/OrderBy.cs
@@ -0,0 +1,28 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Linq;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+namespace RepoTasks
+{
+    public class OrderBy : Task
+    {
+        [Required]
+        [Output]
+        public ITaskItem[] Items { get; set; }
+
+        public string Key { get; set; }
+
+        public override bool Execute()
+        {
+            var key = string.IsNullOrEmpty(Key)
+                ? "Identity"
+                : Key;
+            Items = Items.OrderBy(k => k.GetMetadata(key)).ToArray();
+            return true;
+        }
+    }
+}
diff --git a/build/tasks/RepoTasks.tasks b/build/tasks/RepoTasks.tasks
index e1a43cf51f..1176f79344 100644
--- a/build/tasks/RepoTasks.tasks
+++ b/build/tasks/RepoTasks.tasks
@@ -12,6 +12,7 @@
   <UsingTask TaskName="RepoTasks.CopyPackagesToSplitFolders" AssemblyFile="$(_RepoTaskAssembly)" />
   <UsingTask TaskName="RepoTasks.CreateCommonManifest" AssemblyFile="$(_RepoTaskAssembly)" />
   <UsingTask TaskName="RepoTasks.GenerateRestoreSourcesPropsFile" AssemblyFile="$(_RepoTaskAssembly)" />
+  <UsingTask TaskName="RepoTasks.OrderBy" AssemblyFile="$(_RepoTaskAssembly)" />
   <UsingTask TaskName="RepoTasks.ProcessSharedFrameworkDeps" AssemblyFile="$(_RepoTaskAssembly)" />
   <UsingTask TaskName="RepoTasks.PublishToAzureBlob" AssemblyFile="$(_RepoTaskAssembly)" />
   <UsingTask TaskName="RepoTasks.ReplaceInFile" AssemblyFile="$(_RepoTaskAssembly)" />