New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Branch counter of reporter may have a bug #695
Comments
Probably very related issue with clover renderer - never reached condition is rendered with coverage data from instrumented JS code: currently rendered clover line: <line num="63" count="0" type="cond" truecount="0" falsecount="2"/> expected rendered clover line: <line num="63" count="0" type="cond" truecount="0" falsecount="0"/> this problem is causing CodeCov to think the line is partially covered: the fix should be easy, as the coverage data are correct, probably one condition/line needs to be fixed in the clover renderer code place where rendered:
|
Right. But which practice of clover and CodeCov is more common and standard? |
Addition for this: Test this function export default function (a: number) {
switch (a) {
case 1:
console.log(1)
break
case 2:
console.log(2)
break
case 3:
console.log(3)
break
default:
console.log('default')
}
return a
} Testing file: it("index", () => {
func(1);
}); Clover output: ...
<line num="1" count="1" type="stmt"/>
<line num="2" count="1" type="cond" truecount="1" falsecount="3"/>
<line num="4" count="1" type="stmt"/>
<line num="5" count="1" type="stmt"/>
... Then we change testing file: it("index", () => {
func(1);
func(1);
func(2);
func(99);
}); re-run test and check clover.xml again: ...
<line num="1" count="1" type="stmt"/>
<line num="2" count="4" type="cond" truecount="3" falsecount="1"/>
<line num="4" count="2" type="stmt"/>
...
Changing testing file and source file to other situation, it could be confirmed too. I already add this case at latest commit of my repro. |
@KiritaniAyaka do you have any reference for it? I would say And if a binary expression, the here is some clover output example from our CI: <line num="171" count="56" type="cond" truecount="3" falsecount="1"/>
<line num="177" count="56" type="cond" truecount="1" falsecount="1"/>
<line num="178" count="56" type="stmt"/>
<line num="181" count="0" type="stmt"/>
<line num="185" count="112" type="cond" truecount="2" falsecount="0"/>
|
No reference, just my personal infer. You can check my repro and its CodeCov reports to confirm it. CodeCov and istanbul-reporter has different understanding to |
your example with <?xml version="1.0" encoding="UTF-8"?>
<coverage generated="1665465748533" clover="3.2.0">
<project timestamp="1665465748533" name="All files">
<metrics statements="23" coveredstatements="18" conditionals="8" coveredconditionals="5" methods="4" coveredmethods="4" elements="35" coveredelements="27" complexity="0" loc="23" ncloc="23" packages="1" files="3" classes="3"/>
<file name="index.ts" path="/home/runner/work/istanbul-coverage-report/istanbul-coverage-report/src/index.ts">
<metrics statements="5" coveredstatements="5" conditionals="2" coveredconditionals="2" methods="1" coveredmethods="1"/>
<line num="1" count="1" type="stmt"/>
<line num="3" count="2" type="cond" truecount="2" falsecount="0"/>
<line num="5" count="1" type="stmt"/>
<line num="9" count="1" type="stmt"/>
<line num="12" count="2" type="stmt"/>
</file>
<file name="index2.ts" path="/home/runner/work/istanbul-coverage-report/istanbul-coverage-report/src/index2.ts">
<metrics statements="8" coveredstatements="8" conditionals="2" coveredconditionals="2" methods="2" coveredmethods="2"/>
<line num="1" count="1" type="stmt"/>
<line num="3" count="2" type="stmt"/>
<line num="5" count="2" type="cond" truecount="2" falsecount="0"/>
<line num="7" count="1" type="stmt"/>
<line num="11" count="1" type="stmt"/>
<line num="15" count="2" type="stmt"/>
<line num="17" count="2" type="stmt"/>
<line num="21" count="4" type="stmt"/>
</file>
<file name="index3.ts" path="/home/runner/work/istanbul-coverage-report/istanbul-coverage-report/src/index3.ts">
<metrics statements="10" coveredstatements="5" conditionals="4" coveredconditionals="1" methods="1" coveredmethods="1"/>
<line num="1" count="1" type="stmt"/>
<line num="2" count="1" type="cond" truecount="1" falsecount="3"/>
<line num="4" count="1" type="stmt"/>
<line num="5" count="1" type="stmt"/>
<line num="8" count="0" type="stmt"/>
<line num="9" count="0" type="stmt"/>
<line num="12" count="0" type="stmt"/>
<line num="13" count="0" type="stmt"/>
<line num="16" count="0" type="stmt"/>
<line num="18" count="1" type="stmt"/>
</file>
</project>
</coverage> It seems you are right about the current output format. The output is following:
the CodeCov should then process the coverage like:
@thomasrockhu-codecov can you please comment on this problem? |
@mvorisek I need to dig in further here, but at a glance, Codecov does not consider partial lines as covered. We defined it as To comment on
|
thank you for the reply, the problem is the as shown on a real istanbuljs/nyc clover output data in #695 (comment) istanbuljs/nyc defines CodeCov, per your response, per issue description and also my experience, processes it like number of times the expression has evaluated to false (ie. as there is no official clover standard, I cannot tell which definition is correct, but both make sense so the question is, what CodeCov "usually" receives, are there any other clover reporters that uses the definition implemented by CodeCov? |
Thanks for replying. The logic of CodeCov looks excellent and has no problem. In semantics, I think we should use We can be sure it is a bug of Do you have documentation about BTW, we can't learn from other JavaScript testing libraries because I found almost all of them are using
Well, the implementation of other reporters is also worthy of ref ✨ |
Interesting. I'm aware of istanbuljs/v8-to-istanbul#158, but this is the first I've seen mention of a clover issue. Let me hunt around... edit: On the subject of a schema, I believe that https://gist.github.com/aik099/8556655 is the closest that exists to a documented "standard" |
Thanks for the information, it's very pivotal! <xs:documentation>
...
@count - only applicable if @type == 'stmt' or 'method'; the number of times the construct was executed
@truecount - only applicable if @type == 'cond'; the number of times the true branch was executed
@falsecount - only applicable if @type == 'cond'; the number of times the false branch was executed
...
</xs:documentation> According to this, we can almost know this is a bug of |
Excuse me, I want to discuss further on this. I'm working on this issue but uncertain how to deal with the The I think there are two practices:
Do you have any advice or better practice? |
Hi, I have another reproduction where it is very simple and shows that the I have a branch and am adding lots of testing, there are a number of cases where I get incorrect partial matches, this thought best to start with this issue and give another reproduction for you with 1 very simple file and test. Once the patch is merged I will see how it affects my tests and raise more reproductions of other partial matches should they still exist. I get lots of partial matches on class function calls for example. Thanks :) |
Dropping a comment to get notified on updates |
In your example, are you expecting Presumably, if the |
have same probleam |
### Summary of Changes If the `clover.xml` report is sent to Codecov lines with conditionals are incorrectly marked as only partially covered (see [this issue](istanbuljs/istanbuljs#695)). We are now send the JSON report instead to get a more accurate picture of the code coverage.
Recap
I'm using codecov for code coverage. Codecov marked some lines as
Partial
that means one of branches is not covered, but they are marked as covered in HTML reports.I think codecov is using
clover.xml
as the coverage information source. Then I checked myclover.xml
, the values oftruecount
andfalsecount
seems are wrong, please check my repro and screenshots below.Repro
I created a repo for repro this. My lib is using vitest (vitest depends istanbul indirectly) so I created 2 branches for repro of istanbul and vitest.
HTML Reports
clover.xml
Line 5, 9 are marked as covered in HTML reports, but the
falsecount
of branch at line 3 is0
in xml reports.Codecov
Codecov think one of branches are not covered.
Possible expected output of
clover.xml
I'm new to code coverage, so I'm not sure whether it is a bug of istanbul, or other lib, or codecov?
The text was updated successfully, but these errors were encountered: