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

Why does --all not observe ignore comments? #318

Open
kolton-musgrove opened this issue Jul 21, 2021 · 5 comments
Open

Why does --all not observe ignore comments? #318

kolton-musgrove opened this issue Jul 21, 2021 · 5 comments
Labels
bug help wanted Issue is well defined but needs assignment p2

Comments

@kolton-musgrove
Copy link

If possible, please provide code that demonstrates the problem, keeping it as
simple and free of external dependencies as you are able.

  • Version: 12.22.2
  • Platform: Linux ROG-Zephyrus 5.10.43.3-microsoft-standard-WSL2 Detailed coverage #1 SMP Wed Jun 16 23:47:55 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

When using the --all flag for the c8 report command, any c8 ignore statements are ignored and code inside of them is reported on? From the description of the --all command, (here)[https://github.com/bcoe/c8#checking-for-full-source-coverage-using---all], it seems like the intended use case is when the user wants to get an understanding of how much of their codebase is covered. However, certain files, such as index.js files or files containing simple logic are commonly not tested and do not need to have coverage information. Does it then follow that the way that c8 provides to handle these situations, namely /* c8 ignore */, should then exclude them from any form of coverage reporting?

@JohnAlbin
Copy link

Ah! This explains why my ignore statements aren't working.

I'm using native Node.js ES Modules in my codebase (so no Babel compilation or anything) and my tests thoroughly tested parts of my code base. In other words, some of my codebase was not imported by the tests and totally uncovered.

From Checking for "full" source coverage using --all:

By default v8 will only give us coverage for files that were loaded by the engine. If there are source files in your project that are flexed in production but not in your tests, your coverage numbers will not reflect this. For example, if your project's main.js loads a.js and b.js but your unit tests only load a.js your total coverage could show as 100% for a.js when in fact both main.js and b.js are uncovered.

This was exactly my situation. c8 was reporting 100% coverage but really I only had 100% coverage of 40% of my codebase. I used the --all flag to show the true code coverage. But now I can't use ignore statements.

There's nothing in the docs that suggest a developer would want to turn off the ignore feature so they cannot do this:

/* c8 ignore next */
if (process.platform === 'win32') console.info('hello world')

I like the --all flag because it forces me to add tests to new modules. Otherwise new modules that aren't imported by the rest of the codebase get silently ignored.

Thanks for c8! I'm loving it as a replacement of istanbul/nyc on my ES module codebase.

@JohnAlbin
Copy link

Perhaps this is a bug in the v8-to-istanbul dependency? Ignore comments are handled by that dependency. And I think --all is just merging coverage reports which is also handled by that dependency.

@solomonhawk
Copy link

+1 for this. I also want to use --all to force coverage reporting for all source files but also use /* c8 ignore start */ and /* c8 ignore stop */ to selectively ignore parts of modules (e.g. some lines that are 100% framework code and don't benefit at all from testing and coverage).

@bcoe bcoe added bug p2 help wanted Issue is well defined but needs assignment and removed feature-request labels Nov 11, 2022
@madeleineostoja
Copy link

Any update on whether this is being worked on / if there's a fix in sight? Makes using c8 with a test runner (where coverage is only calculated for tested files without using --all) fairly broken

@AriPerkkio
Copy link
Contributor

This should be fixed in latest v8-to-istanbul: istanbuljs/v8-to-istanbul#217

Repro where temp.js is not run by tests, is included by --all and is fully excluded using ignore hint:

c8-repro
├── index.js
├── package.json
└── temp.js
// index.js
console.log("Hello world");
// temp.js
/* c8 ignore next 3 */
module.exports = function sum(a, b) {
  return a + b;
};
{
  "name": "c8-repro",
  "scripts": {
    "c8": "c8"
  },
  "dependencies": {
    "c8": "^8.0.0"
  }
}
$ pnpm run c8 --all node index.js 

> c8 "--all" "node" "index.js"

Hello world
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |                   
 index.js |     100 |      100 |     100 |     100 |                   
 temp.js  |     100 |      100 |     100 |     100 |                   
----------|---------|----------|---------|---------|-------------------

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted Issue is well defined but needs assignment p2
Projects
None yet
Development

No branches or pull requests

6 participants