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

Code coverage html output. #50

Open
dannysmc95 opened this issue Sep 15, 2023 · 13 comments
Open

Code coverage html output. #50

dannysmc95 opened this issue Sep 15, 2023 · 13 comments

Comments

@dannysmc95
Copy link

Hello!

Less of a bug, more of support issue.

Working with this, wanted to try and get code coverage, and currently seeing this:

ℹ -------------------------------------------------------------------------------
ℹ file                           | line % | branch % | funcs % | uncovered lines
ℹ -------------------------------------------------------------------------------
ℹ src/providers/dot-prop.spec.ts | 100.00 |   100.00 |  100.00 | 
ℹ src/providers/dot-prop.ts      | 100.00 |    86.27 |  100.00 | 
ℹ -------------------------------------------------------------------------------
ℹ all files                      | 100.00 |    90.79 |  100.00 |
ℹ -------------------------------------------------------------------------------

I wanted to use some form of vscode extension to highlight the areas that weren't tested, but I have noticed none seem to support json format only html.

I am currently using this command:

NODE_V8_COVERAGE=coverage node --experimental-test-coverage --report-dir=./src --test-reporter=spec --loader tsx --test ./src/**/*.spec.ts

And it's fine to display it in the console, but how would I get this written to HTML or something else? Are there any built in options?

Thanks in advance.

@aduh95
Copy link
Contributor

aduh95 commented Sep 16, 2023

@nodejs/test_runner

@atlowChemi
Copy link
Member

@philnash has raised an issue suggesting adding a builtin lcov coverage reporter in nodejs/node#49626

AFAIK there are VSCode extensions that can handle lcov reports (for example https://marketplace.visualstudio.com/items?itemName=alexdima.vscode-lcov)
Would this be a viable solution?

@dannysmc95
Copy link
Author

@atlowChemi yeah, the plugin I use with Jest/Vitest https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters works with lcov so that would be fine!

Realistically, I am just looking at a way of simply debugging what things aren't covered in the tests, and for some reason, the uncovered lines - see below, aren't actually outputting anything, I am unable to know what stuff isn't covered by the tests I have written so far.

Adding onto this, as partially related, I am using this command currently

NODE_V8_COVERAGE=coverage node --experimental-test-coverage --report-dir=./src --test-reporter=spec --loader tsx --test ./src/**/*.spec.ts

Which is great, but I get the following output:
image

There are a few issues here:

  1. I have set it to report on all files in ./src folder there are over 40 files in that folder and the ONLY ones that show are the test file, and the file it's importing.
  2. I am not sure why the dot-prop.spec.ts file is showing here, any suggestions to make it run them but not report on them?

@atlowChemi
Copy link
Member

@dannysmc95 What node version do you use when getting this output?
In addition to the issue I mentioned above, @philnash also recently restructured the data in the coverage report in nodejs/node#49320, which was just released in v20.7, which might end changing this output

@philnash
Copy link

I think it still reports coverage on test files too. That’s something that’s been in the back of my mind, as it should not do that, but it’s something the coverage emitter will have to handle rather than the reporter.

@cjihrig
Copy link
Contributor

cjihrig commented Sep 19, 2023

I think it still reports coverage on test files too

Not to side track this issue, but the only reason for that is because people keep opening issues like nodejs/node#45771 and nodejs/node#48956. We would need a solution to those issues. Once a decision is made there, filtering out the test files would be trivial.

The tricker case is when a test suite has something like a helper.js/utils.js file that does not contain any tests but still ends up in the coverage report.

@dannysmc95
Copy link
Author

@dannysmc95 What node version do you use when getting this output?
In addition to the issue I mentioned above, @philnash also recently restructured the data in the coverage report in nodejs/node#49320, which was just released in v20.7, which might end changing this output

Ahh gotcha, I'm pretty sure I'm on 20.6, will have a try and upgrade tomorrow!

@dannysmc95
Copy link
Author

I think it still reports coverage on test files too. That’s something that’s been in the back of my mind, as it should not do that, but it’s something the coverage emitter will have to handle rather than the reporter.

That's fine, I'm more just trying to make sure all files are covered in the reported coverage as otherwise coverage isn't so useful, not too bothered by test files coming up purely because they're all 100% so doesn't affect percentages.

@dannysmc95
Copy link
Author

@dannysmc95 What node version do you use when getting this output? In addition to the issue I mentioned above, @philnash also recently restructured the data in the coverage report in nodejs/node#49320, which was just released in v20.7, which might end changing this output

Looks like the output is still the same unfortunately, no change, other than visual tweaks.
image

@dannysmc95
Copy link
Author

I think it still reports coverage on test files too

Not to side track this issue, but the only reason for that is because people keep opening issues like nodejs/node#45771 and nodejs/node#48956. We would need a solution to those issues. Once a decision is made there, filtering out the test files would be trivial.

The tricker case is when a test suite has something like a helper.js/utils.js file that does not contain any tests but still ends up in the coverage report.

Based on the other issues, I am unsure why you'd want unit tests defined within the source of the code, is this a common thing? Never seen that before.

@cjihrig
Copy link
Contributor

cjihrig commented Sep 20, 2023

I'm personally not a fan of tests in the source code but apparently some people are. My understanding is that it makes testing internals simpler in some cases.

The test runner's code coverage reporting logic could probably be updated to detect the location of tests and filter those out of the results, but it doesn't sound like very much fun to me and I won't be working on it.

@ljharb
Copy link
Member

ljharb commented Sep 20, 2023

There are many, often contradictory, testing patterns out there - i don’t think node should try to support all of them. I think instead node needs to be somewhat opinionated.

Tests in the same file is a pattern in some other languages, but that means you can’t apply test-specific linting rules just to test code, and you can’t use globs to target tests vs production code. I don't think it’s worth supporting.

@dannysmc95
Copy link
Author

dannysmc95 commented Sep 20, 2023

I agree with you @ljharb one of the things that is the hardest with test libraries, are that there aren't many opinions, of course this is mostly a good thing but for things like testing, I would prefer to see a "here are your options" don't like it? Use another library.

If Node can get some of the tweaks I have noted above done, I would be considering using node's testing library for all unit testing going forward, just because it's built-in, no half-maintained 10+ packages I need to install to make it work, and so far with the tsx loader, it has worked pretty well so far, and is a shit ton faster than other libraries out there.

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

6 participants