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

test_runner: lcov reporter for test coverage #49626

Closed
philnash opened this issue Sep 12, 2023 · 13 comments · Fixed by #50018
Closed

test_runner: lcov reporter for test coverage #49626

philnash opened this issue Sep 12, 2023 · 13 comments · Fixed by #50018
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@philnash
Copy link
Contributor

What is the problem this feature will solve?

We can now produce code coverage reports from the built-in test runner and (currently experimental) code coverage. Being able to report that coverage to third-parties, like SonarQube, SonarCloud, Coveralls, or Code Climate is useful to track coverage over time.

Each of these tools accepts lcov as a format for providing test coverage results.

What is the feature you are proposing to solve the problem?

I propose to write a test reporter that outputs code coverage results in the lcov format.

I've actually got quite a long way through this, as part of adding more information to the test:coverage event sent to reporters. I don't wish the reading of the lcov documentation on anyone else.

I did want to raise a feature request to see if there were any comments or thoughts before I open a PR.

What alternatives have you considered?

This could be built as a third-party module, but given lcov's seeming ubiquity as the code coverage format of choice for different code quality tools, it would be useful for users to have it built in.

In comparison with other platforms, Deno can generate an lcov report from its coverage output and Bun has an open issue to implement lcov, so this is becoming par for the course for a test runner.

I am happy to work on this. Full disclosure, I work at Sonar and this would be beneficial to our users if they decide to choose to use the Node test runner in their projects.

@philnash philnash added the feature request Issues that request new features to be added to Node.js. label Sep 12, 2023
@MoLow MoLow added the test_runner Issues and PRs related to the test runner subsystem. label Sep 12, 2023
@MoLow
Copy link
Member

MoLow commented Sep 12, 2023

I think this is a good idea, my only concern is source map support

@gabriel-peracio
Copy link

gabriel-peracio commented Sep 12, 2023

VSCode, as well as many extensions (e.g. ryanluker.vscode-coverage-gutters) also need lcov reports in order to annotate files. Same for bots that post coverage results to PRs.

I support and want this.

@philnash
Copy link
Contributor Author

I think this is a good idea, my only concern is source map support

That's certainly important. But should that be handled before the test:coverage event is fired? That is, should the data that comes in the test:coverage event already have been augmented with data from the source map? It shouldn't be on each reporter to do that work, right?

And if that is the case, a reporter can be built independently of source map support. And the feature only graduates from experimental when it has source map support.

@MoLow
Copy link
Member

MoLow commented Sep 13, 2023

my comment wasn't made to block or object to this feature - just to raise this as something to think about before the feature is implemented. if it is possible to map lcov format after it is generated to its source that works as well

@philnash
Copy link
Contributor Author

Thanks @MoLow, I will certainly keep it in mind.

@fernandopasik
Copy link

This is a fantastic feature to have in node!

As a workaround I've used this for now
https://github.com/bcoe/c8

NODE_V8_COVERAGE=./coverage c8 -r html node --test --experimental-test-coverage src/**/*.spec.ts

philnash added a commit to philnash/node that referenced this issue Oct 2, 2023
philnash added a commit to philnash/node that referenced this issue Oct 3, 2023
philnash added a commit to philnash/node that referenced this issue Oct 23, 2023
nodejs-github-bot pushed a commit that referenced this issue Oct 25, 2023
Fixes #49626

PR-URL: #50018
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
Fixes nodejs#49626

PR-URL: nodejs#50018
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
targos pushed a commit that referenced this issue Nov 11, 2023
Fixes #49626

PR-URL: #50018
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
Fixes #49626

PR-URL: #50018
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@fernandopasik
Copy link

@philnash thank you for adding lcov support!
Would it make sense to raise a feature request to generate the html coverage report too?

@philnash
Copy link
Contributor Author

@fernandopasik there's certainly nothing wrong with raising a feature request for an HTML coverage report.

The only issue I see with it is that it wouldn't necessarily fit with the model of existing reporters, in that you normally define a single output file and an HTML report would require multiple files, normally going into a coverage directory of its own.

@fernandopasik
Copy link

it wouldn't necessarily fit with the model of existing reporters, in that you normally define a single output file

It's a very good point.
I'll raise the feature issue, but tldr the problem I'm trying to resolve is navigating in detail the coverage of files to find the missing tested lines, functions, etc

A note also on this is some of the lcov reporters in other runners like jest and mocha do generate the html output files in the same directory than the lcov.info file.

I tried to mitigate requesting for this by using a package to generate html report from the lcov file, but I was unsuccessful, not sure if you have any good alternatives for this.

@philnash
Copy link
Contributor Author

What package did you try and what was unsuccessful about it? I haven't actually tried that myself yet, but it's something I'd be happy to look into.

@fernandopasik
Copy link

fernandopasik commented Jan 15, 2024

Sorry I should have said it didn't work for my particular problem 😊 I haven't tried it with js files, it didn't work with some typescript project I was working on, but I was hoping you might known an alternative package I could try.

The issue I was having was that I could not explore in detail each file in the lcov.info file

eugenezinovyev/lcov-viewer#89

@philnash
Copy link
Contributor Author

Ah, my understanding is that coverage still doesn't work accurately for TypeScript because the coverage system doesn't yet support source maps.

@fernandopasik
Copy link

fernandopasik commented Jan 27, 2024

Ah, my understanding is that coverage still doesn't work accurately for TypeScript because the coverage system doesn't yet support source maps.

Oh thanks! By any chance do you know an issue in this repo about it that I can follow?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants