MicrosoftPremier / VstsExtensions

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

BuildQualityChecks sums up coverage data of multiple runs when rerun a build #232

Closed mabihz closed 3 months ago

mabihz commented 4 months ago

Describe the context

When a build is started once again (rerun), BQC seems to call in coverage data from all runs and sums up the values (in my case total blocks and covered blocks). This is a problem if the first run failed with a low code coverage (caused by any reason) and the second one should succeed because of a good coverage, but both runs get sumed up and the average is not high enough.

I proofed that, by running a successfull build two times. In the second run, the code coverage percentage was the same, but the blocks were the double of the first run.

If I would know, where the coverage data is called from, maybe I could handle that by cleaning it up. In the end BQC should not sum up coverage data of multiple runs.

mabihz commented 4 months ago

What i have already tried and did not help:

ReneSchumacher commented 3 months ago

Hi @mabihz,

sorry for the late response, I somehow missed this issue. Can you tell me what type of code coverage you are using and how you're publishing coverage? I would like to repro the issue on our side.

There is a built-in coverage aggregation in Azure DevOps, which might lead to this issue if you automatically rerun a job within a pipeline. Once the pipeline has finished and you then manually rerun the job, coverage data shouldn't be aggregated anymore. But I'll have to check this on the Azure DevOps side. BQC simply reads the data from Azure DevOps. Thus, if Azure DevOps aggregates data, there is no way for BQC to know that.

mabihz commented 3 months ago

Hi Rene,

nice to conversate with a countryman, but i keep writing in english for community. In my case the code coverage comes from VsTest as .coverage files. The test results get published via PublishBuildArtifacts task. I also tried, without that publish step, but the problem remained.

Here are the 3 relevant steps from the pipeline:

  - task: VSTest@1
    displayName: 'Test ausführen'
    inputs:
      testAssembly: '**\$(BuildConfiguration)\*test*.dll;-:**\obj\**'
      testFiltercriteria: 'testCategory!=Integration'
      runSettingsFile: ${{parameters.componentSlnPath}}\${{parameters.componentRunSettingsFile}}
      codeCoverageEnabled: true
      runInParallel: False
      platform: '$(BuildPlatform)'
      configuration: '$(BuildConfiguration)'

  - task: PublishBuildArtifacts@1
    displayName: 'CodeCoverage Report'
    inputs:
      PathtoPublish: '$(Build.SourcesDirectory)\TestResults'
      ArtifactName: TestResults

  - task: BuildQualityChecks@8
    displayName: 'Check Code Coverage'
    inputs:
      checkCoverage: true
      coverageFailOption: 'build'
      coverageType: 'blocks'
      allowCoverageVariance: true
      coverageVariance: '0.1'
      buildConfiguration: '$(BuildConfiguration)'
      buildPlatform: '$(BuildPlatform)'
      includePartiallySucceeded: false
      baseDefinitionId: '$(Build.DefinitionName)'
      baseRepoId: '$(Build.Repository.Name)'
      baseBranchRef: '$(System.PullRequest.TargetBranch)'
ReneSchumacher commented 3 months ago

Hi again,

that was my mistake. There is an aggregation running in the backend, but it only works within a single build run and cannot aggregate multiple runs. However, we aggregate coverage data in BQC. The reason is that you may have multiple test runs in your pipeline. If all of them are VSTest, then the code coverage merge job in Azure DevOps itself merges all of these into one aggregated result. However, if you have other test tools/coverage report types in addition to the ones from VSTest (e.g., a Cobertura report from a .NET Core build), the API returns two coverage results. The overall coverage of the build is then the aggregated coverage of all runs.

Now, there cannot be more than one non-VSTest result. If you try publishing multiple Cobertura reports (using the PublishCodeCoverageResults task in version 1.x), only one of them (I believe the first one) is stored in Azure DevOps. To fix this, Azure DevOps release version 2.x of the PublishCodeCoverageResults task, which aggregates all reports locally on the agent before it publishes the aggregated result to Azure DevOps. Still, you migh have one result from VSTest and one from other testing tools, which should be aggregated by BQC.

Unfortunately, when rerunning jobs, VSTest coverage results can be published again, and the API then returns two results (one from each pipeline run). However, since they both belong to the same build (same build ID), there is no way to figure out which run created which result. For BQC the two results look like any other two results created from different testing tools, and, thus, it aggregates them.

Long story short: There's no workaround for this. When you rerun jobs, BQC will always either aggregate coverage data from both runs (when using VSTest) or wrongly use the data from the first run only (for non-VSTest), because you cannot publish another non-VSTest result if one already exists. For now, you would need to run a new build (not rerun a job).

I could add an option to turn off aggregation (default would still be on) to fix your special case. Yet, this only works for builds that solely use VSTest. The moment you want to evaluate coverage for VSTest and another testing tool, aggregation would be needed again. Or you would have to use two BQC tasks with explicit coverage filters and arbitrary values for build platform and build configuration to specifically pick one of the coverage datasets.

What's your reason to rerun the job instead of running a new build? If it's easy for you to move to running another build, I wouldn't change the task. Otherwise, I can quickly add the option and release a new version.

mabihz commented 3 months ago

Thanks for your explanation. Now i understand.

There is no reason for us, to rerun the build. Thats just comfortability. So in the future we will run a new build in that case. Thats okay for us.

Maybe that should be fixed/optimized in Azure DevOps first. Though i think, that could be a problem for anyone using that rerun functionality. Maybe the "turn off aggregation" option will be helpful for anyone.