fix: wrong output truecount and falsecount of clover

[KiritaniAyaka]

close #695

Changes

• fix wrong truecount and falsecount in clover
• switch will be considered a statement now because it has no binary result (not applicable to truecount and falsecount). (#discussion_r1091535324)
• for default args and binary expression, use the fake value of truecount and falsecount to express covered and not covered

[bcoe]

switch will be considered a statement now because it has no binary result

For branch coverage, I'm assuming we'll understand coverage of the branch based on line coverage?

[bcoe]

Are there other branch types we should consider, e.g., ternary operations?

[KiritaniAyaka]

Oh, thanks for point out. I'll dig it further later.

[KiritaniAyaka]

I have done a simple search in source code and a testing (Check it in my repo). Except if and switch, there are three types of branches:

default-arg:
The states always have one member and it presents the number of times that default arguments are applied.

cond-expr: (ternary operator)
The states always have two members. The first presents the number of times that condition was judged as true, and the second presents the number of times that condition was judged as false.

binary-expr:
The number of members is equal to the number of conditions. The xth member presents the number of times that xth condition was calculated. And it has nothing to do with the nest and calculating order.

---

For default-arg, I think it should be considered a statement according to the standard mentioned at #695 .

For cond-expr, it could be deal with the same as if.

I have no good idea to deal with binary-expr. Maybe we can consider it as covered when all conditions are calculated?

[KiritaniAyaka]

I dealt with the remaining branch types in the latest commit. Please check again. @bcoe

[silverbackdan]

Would be a great and very useful fix, I'm in no position to reproduce but have been experiencing this issue with partial matches. Thanks all for the work leading to this PR. In the linked issue I have another simple reproduction if required.

[KiritaniAyaka]

Hi, @bcoe. The problem mentioned in issue #695 looks to make others puzzled now so I think it's needed to push this PR.

I guess this PR stop forward because you may disagree with the practice that assigning hardcode values to make 3rd-party parsers understand if this line was covered or not.

Could we have better practice to resolve this problem?

[silverbackdan]

FYI if anyone finds this thread and is looking for an answer, I've found uploading coverage-final.json instead of clover.xml to codecov results in the correct results and there are not partial line matches galore.

[KiritaniAyaka]

FYI if anyone finds this thread and is looking for an answer, I've found uploading coverage-final.json instead of clover.xml to codecov results in the correct results and there are not partial line matches galore.

Great alternative plan!

Just enable json reporter in .nycrc to resolve it, for example:

{
  "extends": "@istanbuljs/nyc-config-typescript",
  "include": ["src/**/*.ts"],
  "extension": [".ts"],
  "reporter": ["html", "json"], // enable json reporter
  "all": true,
  "sourceMap": true,
  "instrument": true
}

Here's my coverage report on codecov after switch to json reporter

[silverbackdan]

Great, pleased it's helped someone. I found the idea from uploading a different type of report for a different coverage provider so thought it worth a shot. I'd like to my updated coverage to show as well, but it's on a feature branch which will be deleted shortly, but anyway. Happy days.

But this is my PR which has the update showing the coverage increase
components-web-app/cwa-nuxt-module#89

TBH it should be nice if a partial match showed if a conditional statement doesn't match both true and false at some point, but technically it is a hit for the line so coverage that the line was tested is correct.

[bcoe]

Your implementation seems good to me, but it would be helpful to have a comment above each logical check, to explain the decisions you've made.

[bcoe]

What is the effect of adding a line tag here and returning immediately, will this result in 100% line coverage and no missing branch coverage?

[KiritaniAyaka]

It is from the original code:

if (branchDetail) {
    attrs.type = 'cond';
    attrs.truecount = branchDetail.covered;
    attrs.falsecount = branchDetail.total - branchDetail.covered;
}
this.xml.inlineTag('line', attrs);

but the true case is too complex now, so I move the false case into the brackets and change the condition to !branchDetail.

As to branchDetail.type === 'switch'. Thanks to you, I found a situation as you said:

switch (num) {
    case 0:
    case 1:
        console.log('Hello');
        break;
}

When we use num=1 to test this switch, all lines are covered but branches not.

Maybe we should treat switch the same as binary-expr.

[bcoe]

This logic is complex enough, that I think it would be good to add a description above each condition, explaining the reasoning for the logic.