Fix coverage aggregation from multiple reports

[pavel-mikula-sonarsource]

Description

On sonar-dotnet build itself, we produce several coverage reports at the moment, each from a dedicated test project (here are the relevant fragments):

• coverage.DotNetCoverageNet7.net7.0-windows.xml.txt
• coverage.DotNetCoverageNet8.xml.txt
• coverage.DotNetCoverageTestFramework.xml.txt
• coverage.DotNetCoverageNet48.net48.xml.txt

These gets all imported into coverage sensor and processed via our aggregation logic. For the production code in SonarAnalyzer.TestFrameworkVerificationIssueValidationIssueLocationPair.cs and line 68

 else if (Expected is null)

this produces 2 out of 3 covered conditions that don't make sense.

We need to fix the aggregation.

Situation

The affected production code is part of our test framework. It is tested:

• Directly on all paths via coverage.DotNetCoverageTestFramework.xml.txt
• Indirectly, mostly only on happy paths via coverage.DotNetCoverageNet7.net7.0-windows.xml.txt and coverage.DotNetCoverageNet48.net48.xml.txt
• Not at all via coverage.DotNetCoverageNet8.xml.txt, because it doesn't reach this error state in the current set of tests

Coverage data

Coverage data contains expected values like this:

coverage.DotNetCoverageTestFramework.xml.txt

Indicates that there are 2 branches and both are visited

                <SequencePoint vc="70" uspid="198" ordinal="2" offset="54" sl="68" sc="14" el="68" ec="35" bec="2" bev="2" fileid="940" />
...
                <BranchPoint vc="52" uspid="204" ordinal="4" offset="60" sl="68" path="0" offsetend="65" fileid="940" />
                <BranchPoint vc="18" uspid="205" ordinal="5" offset="60" sl="68" path="1" offsetend="236" fileid="940" />

coverage.DotNetCoverageNet7.net7.0-windows.xml.txt and it's net48 counterpart

Indicates that there are 2 branches, and one of them is visited - this is expected.

                <SequencePoint vc="2" uspid="198" ordinal="2" offset="54" sl="68" sc="14" el="68" ec="35" bec="2" bev="1" fileid="940" />
...
                <BranchPoint vc="2" uspid="204" ordinal="4" offset="60" sl="68" path="0" offsetend="65" fileid="940" />
                <BranchPoint vc="0" uspid="205" ordinal="5" offset="60" sl="68" path="1" offsetend="236" fileid="940" />

coverage.DotNetCoverageNet8.xml.txt

Indicates that there are 2 branches and none of them is visited from this test run - this is also expected

                <SequencePoint vc="0" uspid="198" ordinal="2" offset="54" sl="68" sc="14" el="68" ec="35" bec="2" bev="0" fileid="940" />
...
                <BranchPoint vc="0" uspid="204" ordinal="4" offset="60" sl="68" path="0" offsetend="65" fileid="940" />
                <BranchPoint vc="0" uspid="205" ordinal="5" offset="60" sl="68" path="1" offsetend="236" fileid="940" />

[Jetski5822]

Seems I am having this issues too - with dotcover, when importing with net6.0 and net8.0 coverage goes to zero.

[andrei-epure-sonarsource]

The code coverage parsers and importers and aggregators are in https://github.com/SonarSource/sonar-dotnet/tree/master/sonar-dotnet-shared-library/src/main/java/org/sonar/plugins/dotnet/tests

Looking at https://sonarcloud.io/code?id=sonaranalyzer-dotnet&selected=sonaranalyzer-dotnet%3Aanalyzers%2Ftests%2FSonarAnalyzer.TestFramework%2FVerification%2FIssueValidation%2FIssueLocationPair.cs&line=68 at this moment in time, it appears to be fixed...

[sebastien-marichal]

#9018 could have improved coverage results.

[pavel-mikula-sonarsource]

#9018 could have improved coverage results.

I would say "It hid the problem again"

[Tim-Pohlmann]

The problem is not with the aggregation but with AltCover reporting differently for different framework versions:
SteveGilham/altcover#216

This issue is blocked until AltCover's behavior is changed (which might not happen; I'm not sure it's a bug) or we find a better way to match BranchPoints.

[pavel-mikula-sonarsource]

We still know that there are only 2 in any case. And we report 3, so we should be able to improve the behavior. Same branchpoint issue might be relevant for any coverage tool

[Tim-Pohlmann]

@pavel-mikula-sonarsource, can you clarify how we would know that there are only two branch points?

[Tim-Pohlmann]

One thing we could do (and this might be what @pavel-mikula-sonarsource is proposing) is to look at the number of distinct branch points we find per report instead of in total. So we count the number of branches per report and then take the maximum of these counts as the number for all branches.
In the example above, all reports find two branches each, so we would aggregate to two branches total. Funnily enough, this could lead to the opposite problem of what we have now: If all branches are visited in all reports, it would create a 3 out of 2 situation. This could easily be prevented by capping the first number.
There might be some weird edge cases where this could lead to wrong reports: If report R1 finds branchpoint B1 but not B2 and report R2 finds B2 but not B1, we would report only one branch. I'm unsure what kind of code could produce a situation like this.
Overall, while a bit hacky, I expect this to be an improvement over what we have right now. If there are no objections, I will start implementing this next week, considering there is no traction on the AltCover issue.