From fbbca53f4c50322cef48e5c5ab55ddc04d162bcd Mon Sep 17 00:00:00 2001
From: Christoph Bergmeister <c.bergmeister@gmail.com>
Date: Mon, 11 Jun 2018 22:18:51 +0100
Subject: [PATCH] AppVeyor build matrix: more efficient build job split to
 reduce total time by another 5 minutes (#7021)

Closes #6944

Following PR #6945, further, reduce the total build time (without any disadvantages) by around 5 minutes by making sure there is a more even split between the 2 build jobs (the 2nd build job used to be much shorter).
Therefore this PR moves also the xUnit and Pester-Admin tests into the 2nd build job. If it is a daily/feature test commit, then the feature tests will also happen (only) in the 2nd build job. Because both jobs now run tests, the failfast option was removed. The final question from my side is whether running tests in 2 build jobs is OK for the daily build, which uploads code coverage results?

The time to wait for the AppVeyor build results is now 15 +/- 2 minutes, which is a huge improvement to what used to be around 28 minutes before the build matrix was introduced.
---
 appveyor.yml        | 18 +++++------
 tools/appveyor.psm1 | 77 +++++++++++++++++++++++----------------------
 2 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/appveyor.yml b/appveyor.yml
index 4ab026f2c9..11853cf87b 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -5,13 +5,9 @@ environment:
   DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 1
   matrix:
   - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
-    Purpose: BuildingAndPackaging
+    Purpose: UnelevatedPesterTests
   - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017
-    Purpose: BuildingAndTesting
-
-# Stop all jobs in the matrix if any one them fail
-matrix:
-  fast_finish: true
+    Purpose: ElevatedPesterTests_xUnit_Packaging
 
 # cache version - netcoreapp.2.1-sdk.2.1.300
 cache:
@@ -26,14 +22,14 @@ install:
   - ps: Invoke-AppveyorInstall
 
 build_script:
-  - ps: if ($env:Purpose -eq 'BuildingAndTesting'){ Invoke-AppveyorBuild }
+  - ps: Invoke-AppveyorBuild
 
 test_script:
-  - ps: if ($env:Purpose -eq 'BuildingAndTesting'){ Invoke-AppveyorTest }
+  - ps: Invoke-AppveyorTest -Purpose $env:Purpose
 
 after_test:
-  - ps: if ($env:Purpose -eq 'BuildingAndTesting'){ Invoke-AppVeyorAfterTest }
+  - ps: Invoke-AppVeyorAfterTest
 
-# Run the finish command and delete files which should not be in the cache
+# Packaging
 on_finish:
-  - ps: if ($env:Purpose -eq 'BuildingAndPackaging'){ Invoke-AppveyorFinish }
+  - ps: if ($env:Purpose -eq 'ElevatedPesterTests_xUnit_Packaging'){ Invoke-AppveyorFinish }
diff --git a/tools/appveyor.psm1 b/tools/appveyor.psm1
index 483e843307..4299554577 100644
--- a/tools/appveyor.psm1
+++ b/tools/appveyor.psm1
@@ -318,7 +318,10 @@ function Update-AppVeyorTestResults
 function Invoke-AppVeyorTest
 {
     [CmdletBinding()]
-    param()
+    param(
+        [ValidateSet('UnelevatedPesterTests', 'ElevatedPesterTests_xUnit_Packaging')]
+        [string] $Purpose
+    )
     #
     # CoreCLR
 
@@ -333,47 +336,45 @@ function Invoke-AppVeyorTest
         throw "CoreCLR pwsh.exe was not built"
     }
 
-    if(-not (Test-DailyBuild))
-    {
-        # Pester doesn't allow Invoke-Pester -TagAll@('CI', 'RequireAdminOnWindows') currently
-        # https://github.com/pester/Pester/issues/608
-        # To work-around it, we exlude all categories, but 'CI' from the list
-        $ExcludeTag = @('Slow', 'Feature', 'Scenario')
-        Write-Host -Foreground Green 'Running "CI" CoreCLR tests..'
-    }
-    else
-    {
+    # Pester doesn't allow Invoke-Pester -TagAll@('CI', 'RequireAdminOnWindows') currently
+    # https://github.com/pester/Pester/issues/608
+    # To work-around it, we exlude all categories, but 'CI' from the list
+    $ExcludeTag = @('Slow', 'Feature', 'Scenario')
+    if (Test-DailyBuild) {
         $ExcludeTag = @()
         Write-Host -Foreground Green 'Running all CoreCLR tests..'
     }
-
-    Start-PSPester -Terse -bindir $env:CoreOutput -outputFile $testResultsNonAdminFile -Unelevate -Tag @() -ExcludeTag ($ExcludeTag + @('RequireAdminOnWindows'))
-    Write-Host -Foreground Green 'Upload CoreCLR Non-Admin test results'
-    Update-AppVeyorTestResults -resultsFile $testResultsNonAdminFile
-
-    Start-PSPester -Terse -bindir $env:CoreOutput -outputFile $testResultsAdminFile -Tag @('RequireAdminOnWindows') -ExcludeTag $ExcludeTag
-    Write-Host -Foreground Green 'Upload CoreCLR Admin test results'
-    Update-AppVeyorTestResults -resultsFile $testResultsAdminFile
-
-    Start-PSxUnit -SequentialTestResultsFile $SequentialXUnitTestResultsFile -ParallelTestResultsFile $ParallelXUnitTestResultsFile
-    Write-Host -ForegroundColor Green 'Uploading PSxUnit test results'
-    Update-AppVeyorTestResults -resultsFile $SequentialXUnitTestResultsFile
-    Update-AppVeyorTestResults -resultsFile $ParallelXUnitTestResultsFile
-
-    #
-    # Fail the build, if tests failed
-    @(
-        $testResultsNonAdminFile,
-        $testResultsAdminFile
-    ) | ForEach-Object {
-        Test-PSPesterResults -TestResultsFile $_
+    else {
+        Write-Host -Foreground Green 'Running "CI" CoreCLR tests..'
     }
 
-    @(
-        $SequentialXUnitTestResultsFile,
-        $ParallelXUnitTestResultsFile
-    ) | ForEach-Object {
-        Test-XUnitTestResults -TestResultsFile $_
+    if ($Purpose -eq 'UnelevatedPesterTests') {
+        Start-PSPester -Terse -bindir $env:CoreOutput -outputFile $testResultsNonAdminFile -Unelevate -Tag @() -ExcludeTag ($ExcludeTag + @('RequireAdminOnWindows'))
+        Write-Host -Foreground Green 'Upload CoreCLR Non-Admin test results'
+        Update-AppVeyorTestResults -resultsFile $testResultsNonAdminFile
+
+        # Fail the build, if tests failed
+        Test-PSPesterResults -TestResultsFile $testResultsNonAdminFile
+    }
+
+    if ($Purpose -eq 'ElevatedPesterTests_xUnit_Packaging') {
+        Start-PSPester -Terse -bindir $env:CoreOutput -outputFile $testResultsAdminFile -Tag @('RequireAdminOnWindows') -ExcludeTag $ExcludeTag
+        Write-Host -Foreground Green 'Upload CoreCLR Admin test results'
+        Update-AppVeyorTestResults -resultsFile $testResultsAdminFile
+
+        Start-PSxUnit -SequentialTestResultsFile $SequentialXUnitTestResultsFile -ParallelTestResultsFile $ParallelXUnitTestResultsFile
+        Write-Host -ForegroundColor Green 'Uploading PSxUnit test results'
+        Update-AppVeyorTestResults -resultsFile $SequentialXUnitTestResultsFile
+        Update-AppVeyorTestResults -resultsFile $ParallelXUnitTestResultsFile
+
+        # Fail the build, if tests failed
+        Test-PSPesterResults -TestResultsFile $testResultsAdminFile
+        @(
+            $SequentialXUnitTestResultsFile,
+            $ParallelXUnitTestResultsFile
+        ) | ForEach-Object {
+            Test-XUnitTestResults -TestResultsFile $_
+        }
     }
 
     Set-BuildVariable -Name TestPassed -Value True
@@ -385,7 +386,7 @@ function Invoke-AppVeyorAfterTest
     [CmdletBinding()]
     param()
 
-    if(Test-DailyBuild)
+    if (Test-DailyBuild)
     {
         ## Publish code coverage build, tests and OpenCover module to artifacts, so webhook has the information.
         ## Build webhook is called after 'after_test' phase, hence we need to do this here and not in AppveyorFinish.