Regression in 6.2.2 and 6.2.1 of Build Quality Checks?

[andrebriggs]

Possibly related to #57 FYI, I'm using GitHub to host the code repositories.

My config

  - task: mspremier.BuildQualityChecks.QualityChecks-task.BuildQualityChecks@6
    displayName: 'Check build quality'
    inputs:
      checkWarnings: false
      warningFailOption: fixed
      warningFilters: ''
      showStatistics: true
      checkCoverage: true
      coverageFailOption: 'Previous Value'
      coverageType: branches
      buildConfiguration: ''
      buildPlatform: ''
      explicitFilter: true
      baseDefinitionId: 128
      baseBranchRef: refs/heads/master
      runTitle: 'Code Coverage Evaluation'
    condition: and(eq(variables['Agent.JobStatus'], 'Succeeded'), endsWith(variables['Agent.JobName'], 'node_12_x'))

Version 6.1.2

Starting: Check build quality
==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.1.2
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Validating code coverage policy...
Successfully read code coverage data from build.
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
Found baseline build with ID 9446.
Successfully read code coverage data from build.
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
Finishing: Check build quality

Version 6.2.1

Starting: Check build quality
==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.2.1
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Using IdentifierJobResolver
Validating code coverage policy...
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
Cannot read property 'evaluate' of undefined
[ERROR] Cannot read property 'evaluate' of undefined
##[error]Cannot read property 'evaluate' of undefined
##[error]At least one build quality policy was violated. See Build Quality Checks section for more details.
Finishing: Check build quality

Version 6.2.2

Starting: Check build quality
==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.2.2
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Using IdentifierJobResolver
Validating code coverage policy...
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
Cannot read property 'evaluate' of undefined
[ERROR] Cannot read property 'evaluate' of undefined
##[error]Cannot read property 'evaluate' of undefined
##[error]At least one build quality policy was violated. See Build Quality Checks section for more details.
Finishing: Check build quality

[ReneSchumacher]

Hi @andrebriggs,

the YAML configuration is invalid. The coverageFailOption can either be build or fixed but not Previous Value (see https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/WarningsPolicy.md#failoption). I would have thought that the build system would throw an error when you specify an invalid entry in YAML as this clearly violates the allowed options in the task.json. Apparently, it doesn't. We'll add error handling for this in the next update.

To fix the issue, please set the coverageFailOption to build.

[andrebriggs]

@ReneSchumacher thanks. Changing the coverageFailOption to build made the error go away. I now see references to code coverage that aren't quite what I expected. Perhaps you can help me understand.

I have a log from a build here :

Starting: Check build quality
==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.2.2
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Using IdentifierJobResolver
Validating code coverage policy...
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1157
Covered branches: 693
Code Coverage (%): 59.8963
Found baseline build with ID 1144.
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
[SUCCESS] Code coverage policy passed with 59.8963% (693 branches).
Finishing: Check build quality

The code coverage referred to above is 59.8963 and from build 1144 (see coverage report here: https://dev.azure.com/abrig/spk/_build/results?buildId=1144&view=codecoverage-tab)

I'm assuming the 59.8963 value refers to the circled value in the screenshot below (Not rounded up if so)?

If so why that particular value? The overall branch coverage isn't close to that value at all. Moreover the 2nd code coverage value in the build log of 60.1059 doesn't see to correspond with anything in the build from the log (build 1146 here)

I'm probably misunderstanding the instructions but the line [SUCCESS] Code coverage policy passed with 59.8963% (693 branches). from the build log makes it seem like 59.8963 is the current coverage number where it is in fact less than the 60.1059 the log refers to.

My configuration is here:

  - task: mspremier.BuildQualityChecks.QualityChecks-task.BuildQualityChecks@6
    displayName: 'Check build quality'
    inputs:
      checkWarnings: false
      warningFailOption: fixed
      warningFilters: ''
      showStatistics: true
      checkCoverage: true
      coverageFailOption: build
      coverageType: branches
      buildConfiguration: ''
      buildPlatform: ''
      explicitFilter: true
      baseDefinitionId: $(SPK_DEFINITION_ID) # Is 20 from https://dev.azure.com/abrig/spk/_build?definitionId=20&_a=summary
      baseBranchRef: refs/heads/master
      runTitle: 'Code Coverage Evaluation'
    condition: and(eq(variables['Agent.JobStatus'], 'Succeeded'), endsWith(variables['Agent.JobName'], 'node_12_x'))

[andrebriggs]

Another thing @ReneSchumacher. After looking at the documentation again I decided to remove buildConfiguration, buildPlatform, and explicitFilter:

  - task: mspremier.BuildQualityChecks.QualityChecks-task.BuildQualityChecks@6
    displayName: 'Check build quality'
    inputs:
      checkWarnings: false
      warningFailOption: fixed
      warningFilters: ''
      showStatistics: true
      checkCoverage: true
      coverageFailOption: build
      coverageType: branches
      # buildConfiguration: ''
      # buildPlatform: ''
      # explicitFilter: true
      baseDefinitionId: $(SPK_DEFINITION_ID)
      baseBranchRef: refs/heads/master
      runTitle: 'Code Coverage Evaluation'
    condition: and(eq(variables['Agent.JobStatus'], 'Succeeded'), endsWith(variables['Agent.JobName'], 'node_12_x'))

After that I get the same value (59.8963) for coverage in the comparison. I expect the same value since no code was changed but still confused about where the value is coming

==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.2.2
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Using IdentifierJobResolver
Validating code coverage policy...
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1157
Covered branches: 693
Code Coverage (%): 59.8963
Found baseline build with ID 1146.
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1157
Covered branches: 693
Code Coverage (%): 59.8963
[SUCCESS] Code coverage policy passed with 59.8963% (693 branches).
Finishing: Check build quality

[andrebriggs]

@ReneSchumacher Looks like the 59.8963 value is the branch-rate from the cobertura-coverage.xml file. Correct?

<coverage lines-valid="2368" lines-covered="1701" line-rate="0.7182999999999999" branches-valid="1157" branches-covered="693" branch-rate="0.599" timestamp="1578966048486" complexity="0" version="0.1">

This must be an issue with the code coverage HTML generation not matching what is in the cobertura-coverage.xml file. Sorry for the confusion

[ReneSchumacher]

Hi @andrebriggs,

as you already figured out the coverage value is taken from the cobertura coverage file. Or more precisely: our task just queries coverage for a build using the Azure DevOps REST API and Azure DevOps simply takes the coverage data from the coverage result file (in your case cobertura). Unfortunately, the testing tools team decided to use the OSS tool ReportGenerator (see https://github.com/danielpalme/ReportGenerator) to render the code coverage report in the Azure DevOps UI. This tool, however, parses the coverage file and does not use the summary information. While parsing, they get different coverage values.

We already talked to the ReportGenerator team and they are convinced that their strategy is correct. The testing tools team apparently isn't very keen on fixing this issue, and I am trying to figure out how to fix it without having to run code coverage analysis myself. Since cobertura is (afaik) just the format, I guess the issue is in the Istanbul coverage generator. Are you using Istanbul to generate code coverage?

The buildPlatform, buildConfiguration, and explicitFilter parameters don't make any difference in your case, because you are generating coverage data that is not assigned with a specific configuration or platform. Imho, only our VSTest runner adds that information to the coverage data so that you can distinguish between coverage for multiple configurations in a multi-config build.

René

[ReneSchumacher]

Hi @andrebriggs,

I just published v6.2.3 of the task that fixes the exception you mentioned. The task will still fail with a proper error message when the YAML properties are invalid. I'm still trying to figure out why the values differ between ReportGenerator and Cobertura/Istanbul. However, since we cannot fix this behavior in our task, I'm closing the issue but keep digging.

René

[ReneSchumacher]

@andrebriggs: I did a couple evaluations of several Cobertura reports and the corresponding ReportGenerator output and finally found the reason for the different coverage values. Apparently Istanbul does not properly report switch statements in the Cobertura report. Thus, ReportGenerator does not notice branches that are created through switch statement. I have reported this in the ReportGenerator project (see danielpalme/ReportGenerator#289) and will also create an issue in the Istanbul project in hopes that someone will fix it.

[danielpalme]

Unfortunately, the testing tools team decided to use the OSS tool ReportGenerator (see https://github.com/danielpalme/ReportGenerator) to render the code coverage report in the Azure DevOps UI.

The problem is, that the report generated by Instanbul is not consistent. If ReportGenerator would use the summary value (0.8889), then the branches in the code (6 of 6) wouldn't match that result.
So you can't blame ReportGenerator for that!

[ReneSchumacher]

Hey @danielpalme,

I'm not blaming anyone and I have explained the issue here and in the issue I created in your repo. It is still unfortunate that we decided to use ReportGenerator in this case, since our build summary showed the (correct) value from the coverage summary before. Since we switched to ReportGenerator, the coverage value shown in the code coverage section is wrong due to the incorrect data in the Cobertura file.

As I said, I'm going to open an issue with the Istanbul team so we can hopefully fix this. Istanbul apparently has the correct values, since the summary values are all correct. However, someone forgot to put switches and jumps into the report details. The good news is, though, that the data reported by our task is correct :D

So don't worry, ReportGenerator is still a great tool and it wasn't my intention to do you or the tool any harm.

René

[ReneSchumacher]

Just for reference: istanbuljs/istanbuljs#517

[andrebriggs]

Thanks for the follow up @ReneSchumacher. I might have to look into the Istanbul code.

One more question. The log below has the Build Quality Checks passing when the current code coverage is 59.8963 and the previous (baseline) code coverage is 60.1059. I would have expected the task to fail? Perhaps the configuration is incorrect again?

Starting: Check build quality
==============================================================================
Task         : Build Quality Checks
Description  : Breaks a build based on quality metrics like number of warnings or code coverage.
Version      : 6.2.4
Author       : Microsoft Premier Services
Help         : [[Docs]](https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/overview.md)
==============================================================================
SystemVssConnection exists true
Using IdentifierJobResolver
Validating code coverage policy...
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1157
Covered branches: 693
Code Coverage (%): 59.8963
Found baseline build with ID 9446.
Successfully read code coverage data from build.
Evaluating coverage data from 1 filtered code coverage data sets...
Total branches: 1133
Covered branches: 681
Code Coverage (%): 60.1059
[SUCCESS] Code coverage policy passed with 59.8963% (693 branches).
Finishing: Check build quality

Current config

 - task: mspremier.BuildQualityChecks.QualityChecks-task.BuildQualityChecks@6
    displayName: 'Check build quality'
    inputs:
      checkWarnings: false
      warningFailOption: fixed
      warningFilters: ''
      showStatistics: true
      checkCoverage: true
      coverageFailOption: build
      coverageType: branches
      baseDefinitionId: $(SPK_DEFINITION_ID)
      baseBranchRef: refs/heads/master
      runTitle: 'Code Coverage Evaluation'
    condition: and(eq(variables['Agent.JobStatus'], 'Succeeded'), endsWith(variables['Agent.JobName'], 'node_12_x'))

[ReneSchumacher]

Yes, it's a config issue again. The default delta type is absolute (see https://github.com/MicrosoftPremier/VstsExtensions/blob/master/BuildQualityChecks/en-US/CodeCoveragePolicy.md#coverageDeltaType), i.e., the policy is comparing the absolute number of blocks. Your current build has 693 covered blocks, the previous one had 681 covered blocks.

Add coverageDeltaType: percentage to your config and it'll work as you expect.

[ReneSchumacher]

Hey everyone,

I submitted a fix for the coverage values issue: istanbuljs/istanbuljs#518
I hope Corey will accept it so that ReportGenerator starts showing the same values as Cobertura.