Skip to content
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

Lines without statements (empty lines) on them are counted as statements #500

Open
millerick opened this issue Nov 17, 2023 · 6 comments
Open

Comments

@millerick
Copy link

  • Version: v18.14.2
  • Platform:
Darwin BL-mmillerick-NNH946444W 23.0.0 Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000 arm64

I've noticed that c8 does not distinguish between number of lines and number of statements. I switched a large project from nyc to c8, and noticed that both the number of lines and the number of statements dramatically increased as part of the switch. Number of lines I understand due to the differences in how nyc and c8 gather the coverage information, but I did not expect the number of statements to equal the number of lines.
image

I put together a minimal example in
https://github.com/millerick/c8-ts-line-statement-example

The coverage report for it is also committed to that repository, but here is a screenshot of it:
image

Here I do not expect line 4 to be counted as a statement. I also would not expect lines 3 or 7 to be counted as statements, but am less opinionated about that.

I read through c8's documentation and did not find any configuration settings I could change to produce the behavior I expected. Is there something that I am missing? Is there a way to improve this so that only lines with actual code statements on them are counted as statements?

@millerick
Copy link
Author

@bcoe , can you comment on this and whether or not it is possible to change this behavior?

@bcoe
Copy link
Owner

bcoe commented Jan 2, 2024

@millerick I'm not quite sure what my reasoning was for the logic here:

https://github.com/istanbuljs/v8-to-istanbul/blob/b0eb41056feff1863d27a1952cf2441e59e4daa7/lib/v8-to-istanbul.js#L278

I believe a line object is added for each line (correctly) but we also iterate over all these lines turning them into statements. It may be that, if a line has no statements on it, we could skip it.

@millerick
Copy link
Author

It may be that, if a line has no statements on it, we could skip it.

Meaning do not create statements for lines that do not actually have a statement on them?

@millerick
Copy link
Author

I spent some time trying to look at this, and while it is easy to get line 4 to count as neither a statement nor a line, I can't find a way to get it to count as a line but not a statement. I tried looking through other code bases I have that are still using nyc for coverage, and I found examples where the number of lines is less than the number of statements, which is not what I would have expected.

I do think that having empty lines not counted as either lines or statements is more correct than the current behavior. @bcoe , if you agree, I will submit a pull request for that change. However, I am unsure if it is possible to make the total number of lines/statements differ from each other in c8 (which may just be an expected difference with c8 when compared to nyc).

@Quramy
Copy link

Quramy commented Feb 29, 2024

I am encountering the same problem. I use c8 and jest to measure coverage and upload them to Codecov.
The number of total lines measured by c8 is more than that measured by jest. This causes the line coverage measured by jest to be under-represented than it should be.
(here is my project Codecov report)

Is there a way or setting to combine the coverage measured by jest (or any other tool using Istanbul instrumentation) with the coverage measured by c8?

@Quramy
Copy link

Quramy commented Feb 29, 2024

Is there a way or setting to combine the coverage measured by jest (or any other tool using Istanbul instrumentation) with the coverage measured by c8?

For now, my problem is resolved configuring https://jestjs.io/ja/docs/configuration#coverageprovider-string to "v8" .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants