-
Notifications
You must be signed in to change notification settings - Fork 30k
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
More detail from test_runner coverage report #49303
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
added
the
feature request
Issues that request new features to be added to Node.js.
label
Aug 24, 2023
atlowChemi
added
the
test_runner
Issues and PRs related to the test runner subsystem.
label
Aug 24, 2023
As long as it works as well or better than what is currently there, I say go for it. |
philnash
added a commit
to philnash/node
that referenced
this issue
Aug 25, 2023
philnash
added a commit
to philnash/node
that referenced
this issue
Aug 25, 2023
This is a breaking change for the format of test:coverage events. But the test coverage is still experimental, so I don't believe it requires a semver-major bump. Fixes nodejs#49303
philnash
added a commit
to philnash/node
that referenced
this issue
Aug 25, 2023
This is a breaking change for the format of test:coverage events. But the test coverage is still experimental, so I don't believe it requires a semver-major bump. Fixes nodejs#49303
alexfernandez
pushed a commit
to alexfernandez/node
that referenced
this issue
Nov 1, 2023
This is a breaking change for the format of test:coverage events. But the test coverage is still experimental, so I don't believe it requires a semver-major bump. Fixes nodejs#49303 PR-URL: nodejs#49320 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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.
What is the problem this feature will solve?
First, I understand that test coverage is still experimental. I'm hoping to move it in the direction of less experimental! I've been working on reporters for both test executions and then looking into test coverage reporters.
I've been trying to implement both the SonarQube generic test coverage report and then also an lcov-text report. With the current output of the
test:coverage
event for a reporter, there isn't enough detail to produce either of these reports.What is the feature you are proposing to solve the problem?
I propose that the
test:coverage
event produces more data. Namely:As part of this, I would also propose that the array of
uncoveredLineNumbers
is removed, as it is serviced by coverage counts per line.I would like to see each file object in the summary look like the following, with keys for functions, branches and lines:
I have a basic implementation of this, and would be happy to tidy it up for a pull request as well as update the existing reporters to handle the new format. I also wondered whether an lcov-text coverage report would be worth providing out of the box or whether that is better in a userland module?
What alternatives have you considered?
I considered keeping the
uncoveredLineNumbers
array, but that puts more work on report implementors to merge an array of numbers and an array of objects if they want to report in order.There is certainly a consideration that this will be a large object. I considered whether sending the data as tuple style arrays (e.g. for a function
[name, count, line]
, for a line[line,count]
would save memory. It likely would, but I don't know how important that is, so I erred towards developer experience and using objects for the data.The text was updated successfully, but these errors were encountered: