[devops] Make test steps fail if there are any test failures. (#15764)

This makes it possible to re-run tests when they fail (since Azure DevOps only allows re-running failed jobs). It shouldn't affect any release pipelines anymore, because the release pipeline only depends on the job that builds the packages now. This also involved some CI changes, to be able to figure out the last test results when a test step is executed multiple times. Also, the GitHub comment will now state the run attempt (if >1) for each test ([example](https://github.com/xamarin/xamarin-macios/pull/15764#issuecomment-1235891944))
2022-09-07 10:38:23 +02:00 · 2022-09-07 10:38:23 +02:00 · 494e4f306d
--- a/tools/devops/automation/scripts/TestResults.Tests.ps1
+++ b/tools/devops/automation/scripts/TestResults.Tests.ps1
@ -7,10 +7,11 @@ Describe "TestResults tests" {
        $label = "pwsh"
        $resultContext = "tests"
        $jobStatus = "Succeeded"
+        $attempt = 1
    }

    It "is correctly created" {
-        $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext)
+        $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext, $attempt)
        $testResult.ResultsPath | Should -Be $dataPath
        $testResult.TestsJobStatus | Should -Be $jobStatus
        $testResult.Label | Should -Be $label
@ -18,12 +19,12 @@ Describe "TestResults tests" {
    }

    It "is successfull" {
-        $testResult = [TestResults]::new($dataPath, "Succeeded", $label, $resultContext)
+        $testResult = [TestResults]::new($dataPath, "Succeeded", $label, $resultContext, $attempt)
        $testResult.IsSuccess() | Should -Be $true
    }

    It "is failure" {
-        $testResult = [TestResults]::new($dataPath, "Failure", $label, $resultContext)
+        $testResult = [TestResults]::new($dataPath, "Failure", $label, $resultContext, $attempt)
        $testResult.IsSuccess() | Should -Be $false
    }

@ -31,7 +32,7 @@ Describe "TestResults tests" {
        BeforeAll {
            $dataPath = Join-Path -Path $PSScriptRoot -ChildPath "test_data" 
            $dataPath = Join-Path -Path $dataPath -ChildPath "MissingFile.md"
-            $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext)
+            $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext, $attempt)
        }

        It "writes the correct comment." {
@ -53,7 +54,7 @@ Describe "TestResults tests" {
        BeforeAll {
            $dataPath = Join-Path -Path $PSScriptRoot -ChildPath "test_data" 
            $dataPath = Join-Path -Path $dataPath -ChildPath "TestSummary.md"
-            $testResult = [TestResults]::new($dataPath, "", $label, $resultContext)
+            $testResult = [TestResults]::new($dataPath, "", $label, $resultContext, $attempt)
        }

        It "writes the correct comment." {
@ -75,7 +76,7 @@ Describe "TestResults tests" {
        BeforeAll {
            $dataPath = Join-Path -Path $PSScriptRoot -ChildPath "test_data" 
            $dataPath = Join-Path -Path $dataPath -ChildPath "TestSummary.md"
-            $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext)
+            $testResult = [TestResults]::new($dataPath, $jobStatus, $label, $resultContext, $attempt)
        }

        It "writes the correct comment." {
@ -101,7 +102,7 @@ Describe "TestResults tests" {

    Context "error job status" {
        BeforeAll {
-            $testResult = [TestResults]::new($dataPath, "Failure", $label, $resultContext)
+            $testResult = [TestResults]::new($dataPath, "Failure", $label, $resultContext, $attempt)
        }

        It "writes the correct comment." {
@ -125,4 +126,37 @@ Describe "TestResults tests" {
        }
    }

+    Context "new test summmary results" {
+        It "finds the right stuff" {
+            [Environment]::SetEnvironmentVariable("TESTS_JOBSTATUS_LINKER", "yay")
+            [Environment]::SetEnvironmentVariable("TESTS_JOBSTATUS_INTROSPECTION", "nay")
+
+            $testDirectory = Join-Path "." "subdir"
+            New-Item -Path "$testDirectory" -ItemType "directory" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixlinker-1" -Name "TestSummary.md" -Value "SummaryA" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixlinker-2" -Name "TestSummary.md" -Value "SummaryB" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixlinker-200" -Name "TestSummary.md" -Value "SummaryC" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixlinker-3" -Name "TestSummary.md" -Value "SummaryD" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixintrospection-2" -Name "TestSummary.md" -Value "SummaryE" -Force
+            New-Item -Path "$testDirectory/TestSummary-prefixmtouch-3" -Name "TestSummary.md" -Value "SummaryF" -Force
+
+            $labels = "linker;introspection".Split(";")
+            $testResults = New-TestSummaryResults -Path "$testDirectory" -Labels $labels -TestPrefix "prefix"
+
+            Remove-Item -Path $testDirectory -Recurse
+
+            $testResults.count | Should -Be 2
+
+            $testResults[0].Label | Should -Be "linker"
+            $testResults[0].Context | Should -Be " - linker"
+            $testResults[0].ResultsPath | Should -Be "$Env:PWD/subdir/TestSummary-prefixlinker-200/TestSummary.md"
+            $testResults[0].TestsJobStatus | Should -Be "yay"
+
+            $testResults[1].Label | Should -Be "introspection"
+            $testResults[1].Context | Should -Be " - introspection"
+            $testResults[1].ResultsPath | Should -Be "$Env:PWD/subdir/TestSummary-prefixintrospection-2/TestSummary.md"
+            $testResults[1].TestsJobStatus | Should -Be "nay"
+        }
+    }
+
 }
--- a/tools/devops/automation/scripts/TestResults.psm1
+++ b/tools/devops/automation/scripts/TestResults.psm1
@ -5,6 +5,7 @@ class TestResults {
    [string] $TestsJobStatus # the value of the env var that lets us know if the tests passed or not can be null or empty
    [string] $Label
    [string] $Context
+    [int] $Attempt
    hidden [int] $Passed
    hidden [int] $Failed
    hidden [string[]] $NotTestSummaryLabels = @("install-source")
@ -13,13 +14,15 @@ class TestResults {
        [string] $path,
        [string] $status,
        [string] $label,
-        [string] $context
+        [string] $context,
+        [int] $attempt
    ) {
-        Write-Debug "TestsResults::new($path, $status, $label, $context)"
+        Write-Debug "TestsResults::new($path, $status, $label, $context, $attempt)"
        $this.ResultsPath = $path
        $this.TestsJobStatus = $status
        $this.Label = $label
        $this.Context = $context
+        $this.Attempt = $attempt
        $this.Passed = -1
        $this.Failed = -1
    }
@ -37,6 +40,13 @@ class TestResults {
        }
    }

+    [string] GetAttemptText() {
+        if ($this.Attempt -gt 1) {
+            return " [attempt $($this.Attempt)]"
+        }
+        return ""
+    }
+
    [void] WriteComment($stringBuilder) {
        if (-not (Test-Path $this.ResultsPath -PathType Leaf)) {
             $stringBuilder.AppendLine(":fire: Tests failed catastrophically on $($this.Context) (no summary found).")
@ -248,10 +258,11 @@ class ParallelTestsResults {
    [void] PrintSuccessMessage($testResult, $stringBuilder) {
        $downloadInfo = $this.GetDownloadLinks($testResult)
        $result = $testResult.GetPassedTests()
+        $attemptText = $testResult.GetAttemptText()
        if ($result.Passed -eq 0) {
-            $stringBuilder.AppendLine(":warning: $($testResult.Label): No tests selected. $downloadInfo")
+            $stringBuilder.AppendLine(":warning: $($testResult.Label): No tests selected.$attemptText $downloadInfo")
        } else {
-            $stringBuilder.AppendLine(":white_check_mark: $($testResult.Label): All $($result.Passed) tests passed. $downloadInfo")
+            $stringBuilder.AppendLine(":white_check_mark: $($testResult.Label): All $($result.Passed) tests passed.$attemptText $downloadInfo")
        }
    }

@ -282,7 +293,8 @@ class ParallelTestsResults {
            # loop over all results and add the content
            foreach ($r in $failingTests)
            {
-                $stringBuilder.AppendLine("### :x: $($r.Label) tests")
+                $attemptText = $r.GetAttemptText()
+                $stringBuilder.AppendLine("### :x: $($r.Label) tests$attemptText")
                $stringBuilder.AppendLine("")
                # print diff messages if the tests crash or if the tests did indeed fail
                # get the result, if -1, we had a crash, else we print the result
@ -345,9 +357,11 @@ function New-TestResults {
        [string]
        $Label,
        [string]
-        $Context
+        $Context,
+        [int]
+        $Attempt
    )
-    return [TestResults]::new($Path, $Status, $Label, $Context)
+    return [TestResults]::new($Path, $Status, $Label, $Context, $Attempt)
 }

 <#
@ -368,5 +382,73 @@ function New-ParallelTestsResults {
    return [ParallelTestsResults]::new($Results, $Context, $TestPrefix, $VSDropsIndex)
 }

+<#
+    .SYNOPSIS
+        Find test results in a directory
+#>
+function New-TestSummaryResults {
+    param (
+        [string]
+        $Path,
+        [string[]]
+        $Labels,
+        [string]
+        $TestPrefix
+    )
+
+    $testResults = [System.Collections.ArrayList]@()
+    foreach ($label in $Labels) {
+        $label = $label.Replace("-", "_")
+        $environmentVariable = "TESTS_JOBSTATUS_$($label.ToUpper())"
+        $status = [Environment]::GetEnvironmentVariable($environmentVariable)
+
+        Write-Host "Test results for $label is '$status'"
+
+        $testSummaryDirectoryExpression = "$Env:SYSTEM_DEFAULTWORKINGDIRECTORY\Reports\TestSummary-$TestPrefix$label-*"
+
+        # Get the list of directories
+        $directoryFilter = "TestSummary-$TestPrefix$label-*"
+        $testSummaryDirectories = Get-ChildItem -Path $Path -Directory -Filter $directoryFilter
+
+        if ($testSummaryDirectories.length -eq 0) {
+            Write-Host "WARNING: Found no directories matching $directoryFilter for label $label"
+            continue
+        }
+
+        # Compute the attempt # and create a list of (Name, Attempt) entries
+        $attemptDirectories = [System.Collections.ArrayList]@()
+        $computeAttempt = {this.Name.Substring($this.Name.LastIndexOf("-") + 1)}
+        foreach ($dir in $testSummaryDirectories) {
+            $attempt = [int] $dir.Name.Substring($dir.Name.LastIndexOf("-") + 1)
+            $obj = [PSCustomObject]@{
+                Name = $dir
+                Attempt = [int] $attempt
+            }
+            $attemptDirectories += $obj
+        }
+
+        # Sort the list by attempt #
+        $attemptDirectories = $attemptDirectories | Sort-Object -Property "Attempt"
+
+        # Get the last path in the array, that's the last attempt
+        $testSummaryDirectory = $attemptDirectories[$attemptDirectories.count-1]
+        $testAttempt = $testSummaryDirectory.Attempt
+        $testAttemptPath = $testSummaryDirectory.Name
+        $testSummaryPath = Join-Path $testAttemptPath "TestSummary.md"
+
+        Write-Host "Found "$attemptDirectories.count" directories, selected $testSummaryDirectory and final path is $testSummaryPath"
+
+        if (-not (Test-Path -Path $testSummaryPath -PathType Leaf)) {
+            Write-Host "WARNING: Path $testSummaryPath does not exist"
+        }
+
+        $result = New-TestResults -Path $testSummaryPath -Status $status -Label $label -Context "$Env:CONTEXT - $label" -Attempt $testAttempt
+        $testResults += $result
+    }
+
+    return $testResults
+}
+
 Export-ModuleMember -Function New-TestResults
 Export-ModuleMember -Function New-ParallelTestsResults
+Export-ModuleMember -Function New-TestSummaryResults
--- a/tools/devops/automation/templates/mac/build.yml
+++ b/tools/devops/automation/templates/mac/build.yml
@ -230,14 +230,14 @@ steps:

      # We failed, so write to the comment file why we failed.
      Set-Content -Path "$Env:GITHUB_FAILURE_COMMENT_FILE" -Value "$msg"
+
+      exit 1
    } else {
      # We succeeded, so remove the failure comment file.
      Remove-Item -Path "$Env:GITHUB_FAILURE_COMMENT_FILE"
-    }

-    # we set the result to 0, the reason is that release pipelines do not like failing tests,
-    # yet we do have some  flacky ones, this will allow the release to run. The comment will let use know that we failed
-    exit 0  
+      exit 0
+    }
  displayName: 'Run tests'
  timeoutInMinutes: 60
  env:
@ -247,7 +247,6 @@ steps:
    SYSTEM_ACCESSTOKEN: $(System.AccessToken)
    MONO_DEBUG: no-gdb-backtrace
    TEST_BOT: $(Agent.Name)
-  continueOnError: true

 - bash: $(System.DefaultWorkingDirectory)/xamarin-macios/tools/devops/automation/scripts/bash/collect-and-upload-crash-reports.sh
  displayName: 'Collect and upload crash reports'
--- a/tools/devops/automation/templates/tests/publish-html.yml
+++ b/tools/devops/automation/templates/tests/publish-html.yml
@ -54,21 +54,8 @@ steps:
    $DebugPreference = "Continue" # enable debug messages

    $labels = "$Env:LABELS".Split(";")
-    $testResults = [System.Collections.ArrayList]@()
-    foreach ($label in $labels) {
-      $label = $label.Replace("-", "_")
-      $status = Get-Item "Env:TESTS_JOBSTATUS_$($label.ToUpper())"
-      $testSummaryPath = "$Env:SYSTEM_DEFAULTWORKINGDIRECTORY\Reports\TestSummary-${{ parameters.testPrefix }}$label\TestSummary.md"

-      Write-Host "Test results for $label $($status.Value)"
-      Write-Host "Test summary path for $label $($testSummaryPath)"
-      if (-not (Test-Path -Path $testSummaryPath -PathType Leaf)) {
-        Write-Host "WARNING: Path is not present."
-      }
-
-      $result = New-TestResults -Path $testSummaryPath -Status $status.Value -Label $label -Context "$Env:CONTEXT - $label"
-      $testResults.Add($result)
-    }
+    $testResults = New-TestSummaryResults -Path "$Env:SYSTEM_DEFAULTWORKINGDIRECTORY\Reports" -Labels $labels -TestPrefix "${{ parameters.testPrefix }}"

    $parallelResults = New-ParallelTestsResults -Results $testResults -Context "$Env:CONTEXT" -TestPrefix "${{ parameters.testPrefix }}" -VSDropsIndex $vsdropsIndex
    $success = $parallelResults.IsSuccess()
--- a/tools/devops/automation/templates/tests/run-tests.yml
+++ b/tools/devops/automation/templates/tests/run-tests.yml
@ -177,16 +177,15 @@ steps:
    # uri used to create the vsdrops index using full uri
    export VSDROPS_URI="$VSDROPSPREFIX/$BUILD_BUILDNUMBER/$BUILD_BUILDID/$PARAMETERS_TESTPREFIX;/tests/"

-    make -C builds download -j || true
-    make -C builds downloads -j || true
-    make -C builds .stamp-mono-ios-sdk-destdir -j || true
-    RC=0
-    make -C tests ${{ parameters.makeTarget }} || RC=$?
-    if [ $RC -eq 0 ]; then
-      echo "##vso[task.setvariable variable=TESTS_JOBSTATUS;isOutput=true]Succeeded"
-    else
-      echo "##vso[task.setvariable variable=TESTS_JOBSTATUS;isOutput=true]Failed"
-    fi
+    # assume something is going to fail
+    echo "##vso[task.setvariable variable=TESTS_JOBSTATUS;isOutput=true]Failed"
+
+    make -C builds download -j
+    make -C builds .stamp-mono-ios-sdk-destdir -j
+    make -C tests ${{ parameters.makeTarget }}
+
+    # We reached the end! This means we succeeded!
+    echo "##vso[task.setvariable variable=TESTS_JOBSTATUS;isOutput=true]Succeeded"
  env:
    TEST_PREFIX: ${{ upper(parameters.testPrefix) }}
    TESTS_EXTRA_ARGUMENTS: ${{ parameters.testsLabels }}
@ -223,18 +222,21 @@ steps:
  condition:  and(eq(variables['system.debug'], true), succeededOrFailed())
  continueOnError: true

-# set the status of the test results. Do not add a comment yet since we have not uploaded any of the needed files for the commnet, this way
+# set the status of the test results. Do not add a comment yet since we have not uploaded any of the needed files for the comment, this way
 # we report the result as soon as we have it and ensure that we set the status from pending to the appropiate value.

 - pwsh: |
    Import-Module $Env:SYSTEM_DEFAULTWORKINGDIRECTORY\xamarin-macios\tools\devops\automation\scripts\MaciosCI.psd1

    $testResultsPath = "$Env:SYSTEM_DEFAULTWORKINGDIRECTORY/xamarin-macios/tests/TestSummary.md"
-    $testResult = New-TestResults -Path $testResultsPath -Status "$(runTests.TESTS_JOBSTATUS)" -Context "${{ parameters.statusContext }}"
+    $testAttempt = [int]"$(System.JobAttempt)"
+    $testResult = New-TestResults -Path $testResultsPath -Status "$(runTests.TESTS_JOBSTATUS)" -Context "${{ parameters.statusContext }}" -Attempt $testAttempt

    $statuses = New-GitHubStatusesObject -Org "xamarin" -Repo "xamarin-macios" -Token $(GitHub.Token)
    $statuses.SetStatus($testResult.GetStatus())
  displayName: "Set tests status."
+  continueOnError: true
+  condition: succeededOrFailed()
  timeoutInMinutes: 5

 # Upload TestSummary as an artifact.
@ -242,7 +244,7 @@ steps:
  displayName: 'Publish Artifact: TestSummary'
  inputs:
    targetPath: 'xamarin-macios/tests/TestSummary.md'
-    artifactName: TestSummary-${{ parameters.testPrefix }}
+    artifactName: TestSummary-${{ parameters.testPrefix }}-$(System.JobAttempt)
  continueOnError: true
  condition: succeededOrFailed()

@ -253,16 +255,20 @@ steps:
      Write-Host "##vso[task.addattachment type=Distributedtask.Core.Summary;name=$summaryName;]$summaryPath"
    }
  displayName: Set TestSummary
+  continueOnError: true
+  condition: succeededOrFailed()

 - task: artifactDropTask@1
  displayName: 'Publish to Artifact Services Drop'
  inputs:
    dropServiceURI: 'https://devdiv.artifacts.visualstudio.com/DefaultCollection'
    dropMetadataContainerName: 'DropMetadata-${{ parameters.testPrefix }}${{ parameters.label }}'
-    buildNumber: 'xamarin-macios/device-tests/$(Build.BuildNumber)/$(Build.BuildId)/${{ parameters.testPrefix }}'
+    buildNumber: 'xamarin-macios/device-tests/$(Build.BuildNumber)/$(Build.BuildId)-$(System.JobAttempt)/${{ parameters.testPrefix }}'
    sourcePath: 'xamarin-macios/jenkins-results'
    detailedLog: true
    usePat: true
+  continueOnError: true
+  condition: succeededOrFailed()

 # Upload test results to vsts.
 - task: PublishTestResults@2