MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
56 stars 14 forks source link

BuildQualityChecks@9 Does Not Appear To Respect Rerun Attempts #224

Closed aolszowka closed 5 months ago

aolszowka commented 8 months ago

Describe the context

Describe the problem and expected behavior

When a pipeline is Reran (Rerun failed jobs) BuildQualityChecks@9 does not appear to account for the most recent Attempt when calculating code coverage.

Screenshots to Follow:

image

image

In our production pipeline, we are being affected by what we believe to be a karama-reporter bug (not yet reported/evaluated), wherein, in scenarios not yet understood, we fail to get a code coverage report generated from our Angular Unit Tests.

This causes an issue in our pipeline because we combine the output of the coverage.cobertura.xml reports from both our Angular and .NET Core API to form a single report that is reported on for the purposes of this gate.

Because there is a significant amount of variance between these two, when the Angular report fails to generate, we fail code coverage.

While we await research into the underlying bug, our developers have attempted to use the Rerun failed jobs to reattempt the build:

image

However, we believe that BuildQualityChecks@9 does not properly account for reruns, as we believe we are seeing the previous failing coverage numbers in its output. Because of this the pipeline will still continue to fail, even if the missing coverage report "shows up" on a subsequent Attempt.

This scenario can be reproduced by attempting to combine any two sets of code coverage and is not Angular specific.

Consider the following toy project layout (a zipped version is attached to this report and will also be emailed to the address provided):

Project Layout

|   azurepipeline.yml
|   CodeCoverageCheck.sln
|
+---CodeCoverageCheck
|       CodeCoverageCheck.csproj
|       CodeCoverageToy.cs
|
+---CodeCoverageCheck.Tests
|       CodeCoverageCheck.Tests.csproj
|       CodeCoverageToyTests.cs
|
+---CodeCoverageCheck.Tests2
|       CodeCoverageCheck.Tests2.csproj
|       CodeCoverageToy2Tests.cs
|
\---CodeCoverageCheck2
        CodeCoverageCheck2.csproj
        CodeCoverageToy2.cs

azurepipeline.yml

# This will be triggered by the gates
trigger: none

pool:
  vmImage: ubuntu-latest

steps:
  - task: DotNetCoreCLI@2
    displayName: 'dotnet test'
    inputs:
      command: 'test'
      projects: CodeCoverageCheck.sln
      arguments: '--collect:"XPlat Code Coverage"'
      publishTestResults: true

################################################################################
# BEGIN HACK
#
# Due to limitations in the PublishCodeCoverageResults@1 [1] that only allow you
# to publish a single code coverage file; we must use a third party tool
# ReportGenerator [2] to combine the results of the previous cobertura runs.
#
# We would like to use PublishCodeCoverageResults@2, which has support to
# combine reports built into it. However we are affected by this bug [3] which
# prevents our code coverage gates from working when using this version.
#
# [1]-https://github.com/microsoft/azure-pipelines-tasks/issues/10353
# [2]-https://github.com/danielpalme/ReportGenerator
# [3]-https://github.com/MicrosoftPremier/VstsExtensions/issues/223
################################################################################

  - task: DotNetCoreCLI@2
    displayName: 'Install ReportGenerator'
    inputs:
      command: custom
      custom: tool
      arguments: 'install --global dotnet-reportgenerator-globaltool'

  - task: PowerShell@2
    displayName: 'Simulate a Unit Test Reporting Failure at Random'
    inputs:
      targetType: 'inline'
      script: |
        $tempDirectory = '$(Agent.TempDirectory)'
        $allCodeCoverageFiles = Get-ChildItem -Path $tempDirectory -Filter coverage.cobertura.xml -Recurse
        Write-Host "There are [$($allCodeCoverageFiles.Length)] coverage files"
        [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]$uniqueItems = [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]::new()
        foreach ($coverageFile in $allCodeCoverageFiles) {
            $currentHash = $coverageFile | Get-FileHash
            if (-Not($uniqueItems.ContainsKey($currentHash.Hash))) {
                $uniqueItems.Add($currentHash.Hash, [System.Collections.Generic.List[string]]::new()) | Out-Null
            }
            $uniqueItems[$currentHash.Hash].Add($currentHash.Path) | Out-Null
        }

        Write-Host "There are [$($uniqueItems.Count)] unique coverage files as follows"
        foreach ($uniqueCoverageFile in $uniqueItems.GetEnumerator()) {
            Write-Host "$($uniqueCoverageFile.Key) -> $([string]::Join(',', $uniqueCoverageFile.Value))"
        }

        $firstUniqueCoverageFile = $uniqueCoverageFile | Select-Object -First 1
        Write-Host "Identified these files for deletion [$($firstUniqueCoverageFile.Value)]"
        if ((Get-Date).Second % 2 -eq 0) {
            Write-Host "Deleting the files because we were on an even second"
            $firstUniqueCoverageFile.Value | Remove-Item
        }

        $allCodeCoverageFiles = Get-ChildItem -Path $tempDirectory -Filter coverage.cobertura.xml -Recurse
        Write-Host "There are [$($allCodeCoverageFiles.Length)] coverage files after this operation"
        [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]$uniqueItems = [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]::new()
        foreach ($coverageFile in $allCodeCoverageFiles) {
            $currentHash = $coverageFile | Get-FileHash
            if (-Not($uniqueItems.ContainsKey($currentHash.Hash))) {
                $uniqueItems.Add($currentHash.Hash, [System.Collections.Generic.List[string]]::new()) | Out-Null
            }
            $uniqueItems[$currentHash.Hash].Add($currentHash.Path) | Out-Null
        }

        Write-Host "There are [$($uniqueItems.Count)] unique coverage files after this operation as follows"
        foreach ($uniqueCoverageFile in $uniqueItems.GetEnumerator()) {
            Write-Host "$($uniqueCoverageFile.Key) -> $([string]::Join(',', $uniqueCoverageFile.Value))"
        }
      pwsh: true

  - task: PowerShell@2
    displayName: 'Use ReportGenerator to Combine Cobertura Results'
    inputs:
      targetType: 'inline'
      script: 'reportgenerator "-reports:$(Agent.TempDirectory)/**/coverage.cobertura.xml" "-targetdir:$(Build.SourcesDirectory)" -reporttypes:Cobertura'
      pwsh: true

  # Last PublishCodeCoverageResults wins; this assumes it is the last one ran
  - task: PublishCodeCoverageResults@1
    displayName: 'publish code coverage (combined)'
    condition: succeededOrFailed()
    inputs:
      codeCoverageTool: Cobertura
      summaryFileLocation: '$(Build.SourcesDirectory)/Cobertura.xml'
      failIfCoverageEmpty: true

################################################################################
# END HACK
################################################################################

  # See https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/CodeCoveragePolicy.md
  - task: BuildQualityChecks@9
    displayName: 'Code Coverage Gate'
    inputs:
      checkCoverage: true
      coverageFailOption: build
      coverageType: line
      treat0of0as100: false
      forceCoverageImprovement: false

CodeCoverageCheck.sln


Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.6.33815.320
MinimumVisualStudioVersion = 10.0.40219.1
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "CodeCoverageCheck", "CodeCoverageCheck\CodeCoverageCheck.csproj", "{04596B9F-BE13-4FC3-ABFA-6B633F151487}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "CodeCoverageCheck.Tests", "CodeCoverageCheck.Tests\CodeCoverageCheck.Tests.csproj", "{D309ECCB-1FAB-480C-B2AC-E0A7064B759B}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CodeCoverageCheck2", "CodeCoverageCheck2\CodeCoverageCheck2.csproj", "{83D8A43C-D1DF-4775-90A3-C513755982AD}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CodeCoverageCheck.Tests2", "CodeCoverageCheck.Tests2\CodeCoverageCheck.Tests2.csproj", "{4E49ADC7-8E2B-450A-BCE1-53EEACAF9227}"
EndProject
Global
        GlobalSection(SolutionConfigurationPlatforms) = preSolution
                Debug|Any CPU = Debug|Any CPU
                Release|Any CPU = Release|Any CPU
        EndGlobalSection
        GlobalSection(ProjectConfigurationPlatforms) = postSolution
                {04596B9F-BE13-4FC3-ABFA-6B633F151487}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
                {04596B9F-BE13-4FC3-ABFA-6B633F151487}.Debug|Any CPU.Build.0 = Debug|Any CPU
                {04596B9F-BE13-4FC3-ABFA-6B633F151487}.Release|Any CPU.ActiveCfg = Release|Any CPU
                {04596B9F-BE13-4FC3-ABFA-6B633F151487}.Release|Any CPU.Build.0 = Release|Any CPU
                {D309ECCB-1FAB-480C-B2AC-E0A7064B759B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
                {D309ECCB-1FAB-480C-B2AC-E0A7064B759B}.Debug|Any CPU.Build.0 = Debug|Any CPU
                {D309ECCB-1FAB-480C-B2AC-E0A7064B759B}.Release|Any CPU.ActiveCfg = Release|Any CPU
                {D309ECCB-1FAB-480C-B2AC-E0A7064B759B}.Release|Any CPU.Build.0 = Release|Any CPU
                {83D8A43C-D1DF-4775-90A3-C513755982AD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
                {83D8A43C-D1DF-4775-90A3-C513755982AD}.Debug|Any CPU.Build.0 = Debug|Any CPU
                {83D8A43C-D1DF-4775-90A3-C513755982AD}.Release|Any CPU.ActiveCfg = Release|Any CPU
                {83D8A43C-D1DF-4775-90A3-C513755982AD}.Release|Any CPU.Build.0 = Release|Any CPU
                {4E49ADC7-8E2B-450A-BCE1-53EEACAF9227}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
                {4E49ADC7-8E2B-450A-BCE1-53EEACAF9227}.Debug|Any CPU.Build.0 = Debug|Any CPU
                {4E49ADC7-8E2B-450A-BCE1-53EEACAF9227}.Release|Any CPU.ActiveCfg = Release|Any CPU
                {4E49ADC7-8E2B-450A-BCE1-53EEACAF9227}.Release|Any CPU.Build.0 = Release|Any CPU
        EndGlobalSection
        GlobalSection(SolutionProperties) = preSolution
                HideSolutionNode = FALSE
        EndGlobalSection
        GlobalSection(ExtensibilityGlobals) = postSolution
                SolutionGuid = {5D7FE5F3-8CCA-4BE1-ABDD-A6DB744CBD3E}
        EndGlobalSection
EndGlobal

CodeCoverageCheck.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

CodeCoverageToy.cs

namespace CodeCoverageCheck
{
    public class CodeCoverageToy
    {
        public static string ToyFunction(int input)
        {
            if(input == 1)
            {
                return "Case 1";
            }
            else if(input == 2)
            {
                return "Case 2";
            }
            else if(input == 3)
            {
                return "Case 3";
            }
            else if (input == 4)
            {
                return "Case 4";
            }
            else
            {
                return "All Other Cases";
            }
        }
    }
}

CodeCoverageCheck.Tests.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="coverlet.collector" Version="6.0.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
    <PackageReference Include="xunit" Version="2.5.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.1">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\CodeCoverageCheck\CodeCoverageCheck.csproj" />
  </ItemGroup>

</Project>

CodeCoverageToyTests.cs

using Xunit;

namespace CodeCoverageCheck.Tests
{
    public class CodeCoverageToyTests
    {
        [Fact]
        public void CodeCoverageToy_ToyFunction_Case1()
        {
            string actual = CodeCoverageToy.ToyFunction(1);
            Assert.Equal("Case 1", actual);
        }
    }
}

CodeCoverageCheck.Tests2.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="coverlet.collector" Version="6.0.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
    <PackageReference Include="xunit" Version="2.5.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.1">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\CodeCoverageCheck2\CodeCoverageCheck2.csproj" />
  </ItemGroup>

</Project>

CodeCoverageToy2Tests.cs

using CodeCoverageCheck2;
using Xunit;

namespace CodeCoverageCheck.Tests2
{
    public class CodeCoverageToy2Tests
    {
        [Fact]
        public void CodeCoverageToy_AnotherToyFunction_Case1()
        {
            string actual = CodeCoverageToy2.AnotherToyFunction(1);
            Assert.Equal("Case 1", actual);
        }
    }
}

CodeCoverageCheck2.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

CodeCoverageToy2.cs

namespace CodeCoverageCheck2
{
    public class CodeCoverageToy2
    {
        public static string AnotherToyFunction(int input)
        {
            if (input == 1)
            {
                return "Case 1";
            }
            else
            {
                return "All Other Cases";
            }
        }
    }
}

This can be a tricky to reproduce, I will attempt to describe how the above toy project works, in order to reproduce this scenario in a clean environment.

In order to simulate a "reporting failure" the following PowerShell Snippet is ran prior to combining the reports:

$tempDirectory = '$(Agent.TempDirectory)'
$allCodeCoverageFiles = Get-ChildItem -Path $tempDirectory -Filter coverage.cobertura.xml -Recurse
Write-Host "There are [$($allCodeCoverageFiles.Length)] coverage files"
[System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]$uniqueItems = [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]::new()
foreach ($coverageFile in $allCodeCoverageFiles) {
    $currentHash = $coverageFile | Get-FileHash
    if (-Not($uniqueItems.ContainsKey($currentHash.Hash))) {
        $uniqueItems.Add($currentHash.Hash, [System.Collections.Generic.List[string]]::new()) | Out-Null
    }
    $uniqueItems[$currentHash.Hash].Add($currentHash.Path) | Out-Null
}

Write-Host "There are [$($uniqueItems.Count)] unique coverage files as follows"
foreach ($uniqueCoverageFile in $uniqueItems.GetEnumerator()) {
    Write-Host "$($uniqueCoverageFile.Key) -> $([string]::Join(',', $uniqueCoverageFile.Value))"
}

$firstUniqueCoverageFile = $uniqueCoverageFile | Select-Object -First 1
Write-Host "Identified these files for deletion [$($firstUniqueCoverageFile.Value)]"
if ((Get-Date).Second % 2 -eq 0) {
    Write-Host "Deleting the files because we were on an even second"
    $firstUniqueCoverageFile.Value | Remove-Item
}

$allCodeCoverageFiles = Get-ChildItem -Path $tempDirectory -Filter coverage.cobertura.xml -Recurse
Write-Host "There are [$($allCodeCoverageFiles.Length)] coverage files after this operation"
[System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]$uniqueItems = [System.Collections.Generic.Dictionary[string, [System.Collections.Generic.List[string]]]]::new()
foreach ($coverageFile in $allCodeCoverageFiles) {
    $currentHash = $coverageFile | Get-FileHash
    if (-Not($uniqueItems.ContainsKey($currentHash.Hash))) {
        $uniqueItems.Add($currentHash.Hash, [System.Collections.Generic.List[string]]::new()) | Out-Null
    }
    $uniqueItems[$currentHash.Hash].Add($currentHash.Path) | Out-Null
}

Write-Host "There are [$($uniqueItems.Count)] unique coverage files after this operation as follows"
foreach ($uniqueCoverageFile in $uniqueItems.GetEnumerator()) {
    Write-Host "$($uniqueCoverageFile.Key) -> $([string]::Join(',', $uniqueCoverageFile.Value))"
}

Which at a high level performs the following:

  1. Grab the $(Agent.TempDirectory) from the Pipeline Run
  2. For reasons not understood, the code coverage results will duplicate the output of this file into a folder in the form of /home/vsts/work/_temp/_fv-az428-823_2023-10-24_22_06_42/In/fv-az428-823/ where fv-az428-823 is the name of the Microsoft Hosted Build Agent assigned at Random, along with the timestamp of when the execution happened.
    • Because of this, in-order for our logic to work, we need to remove a "complete set" of run outputs. Failure to do so will still allow the duplicated report to combine. To do this we take a listing of all the coverage.cobertura.xml files, create a lookup dictionary based on the hash of these files to help us understand the sets of files because we cannot rely on file names.
  3. At "Random", as defined by the current second in which the operation performs is even, drop the first "set" of code coverage files.
    • In Testing this appears to have been the CodeCoverageCheck.Tests2 test results reliably, however we do not believe there is a guarantee. It is hoped that the above coverage files are unbalanced enough to reproduce the scenario in all cases.
  4. The remainder of the code is simply debugging logic to help us understand if the above logic is executing correctly.

To the reproduce the behavior after creating a pipeline one must:

  1. Get a successful run that includes both code coverage files
    • This will produce a baseline for which BuildQualityChecks@9 will compare against.
    • In the Toy Project this means that the code coverage will report as 43.4783% (10/23 line)
  2. Run a NEW pipeline run wherein the execution "fails" (that is that the above PowerShell Logic is triggered to delete one of the outputs).
    • This causes the BuildQualityChecks@9 to fail with 31.25% (5/16 line)
  3. When this occurs you need to select Rerun failed jobs on this EXISTING pipeline (do not run a new pipeline!). You may need to run this several times until you get a "successful" run that DOES NOT trigger the above PowerShell Logic (IE preserves all runs).

A reproducing scenario might look something like this:

image

The Failed Pipeline Reruns will then look like this:

image

When in this state you can now validate that BuildQualityChecks@9 appears to use only the initial run, as opposed to the additional Attempts.

Work Around

Running a NEW pipeline (not Rerun failed jobs) will properly report, however this can be painful depending on the length of the pipeline (because it reruns all jobs, not just the failed ones).

Task logs

I have held off providing these in lieu of providing the entire reproducing project above. Should you need these logs I would be happy to provide them.

ReneSchumacher commented 7 months ago

Hi @aolszowka,

thanks for reporting this and for providing a repro. I'm working on this right now to see what's going on here.

ReneSchumacher commented 7 months ago

Hi again,

it took a while to make the repro work, because the simulation script mostly deleted the coverage from the first component, resulting in a higher coverage value. However, I could repro the issue and is caused by the way Azure DevOps handles coverage data.

Even though you keep publishing newer coverage files with each re-run, Azure DevOps only takes the first published file into account when it comes to the API. You can also see this on the build summary page, which still shows the old value. In the picture below, you see that I had two re-runs, and both (attempt 2 and 3) created increased coverage: image

However, the summary page still shows the reduced coverage value: image

Even though the code coverage tab, which is generated based on the latest published Cobertura file, shows the correct increased value: image

In other words, unless we switch to custom coverage parsing in the BQC task, we cannot change that behavior. BQC just reads the values through the coverage API and it reports the wrong values (in this case).

aolszowka commented 7 months ago

@ReneSchumacher Thank you for looking into this.

However, the summary page still shows the reduced coverage value

This sounds like an Azure DevOps Bug, and at least the second one I've found on this specific page (the other is this one which I have been going around with Microsoft Support on for a few months now: https://github.com/MicrosoftDocs/azure-devops-yaml-schema/issues/212).

Is there any more direct link to the Team's Program Manager? Historically Donovan Brown sat up in build (I think it was //Build2016?) and proudly declared that if we had issues like this to hit him up on Twitter. Who is the current PM? Are they willing to have a conversation? I'd be happy to jump on the phone with them (we're a paying customer!).

For now we'll use the work around and we appreciate your efforts on this extension. I'm willing to have this issue closed out if you want. I wish we could fix some of the underlying issues and make everyone's life easier!

ReneSchumacher commented 7 months ago

I'll see who's responsible for this part and try to connect you with the right contact. Since this is code coverage, I believe it should be the testing tools team, not the build/pipelines team. Will get back to you via email, as I don't want to share internal contact information broadly here in the issue 😉

aolszowka commented 6 months ago

@ReneSchumacher I hope you've had a wonderful Holiday Season, were you able to connect with someone? I can be reached at my username at gmail.

ReneSchumacher commented 5 months ago

@aolszowka Thanks for pinging me again. I haven't received any feedback from PG yet, so I have now asked again. I have lost track of the topic over the holiday season and maybe so has the PG.