Feature: c8 --all

[coreyfarrell]

nyc --all is widely used, an option for c8 to accomplish the same will be needed before many projects are willing to migrate.

Half crazy though: should all: true be the default, c8 --no-all to disable it?

[bcoe]

@coreyfarrell I don't hate the idea of --all being default ... I wonder what --all looks like without require.extension -- unfortunately it's harder for us to tear out non-JavaScript code, so we'd need the instrumentation tool chain to be configured before --all is called.

[j03m]

As an approach to implementing –all for c8 I was considering the following:

• Add an –all flag as Boolean to parse-args.js, propagate this to lib/report.js
• When all is supplied to the report command, glob files in cwd (or --resolve if supplied) respecting flags –include and –exclude. (Seems like test-exclude's globSync api will do this for me)
• Iterate over each of the files and create an empty Istanbul coverage map. I think this can be accomplished using istanbul-lib-instrument’s instrumenter and readInitialCoverage api’s ala something like:

_getEmpyCoverageResultForFile(file){
    const fullPath = resolve(this.resolve, file);
    const code = readFileSync(fullPath).toString();

    const instrumenter = createInstrumenter();
    const instrumentedCode = instrumenter.instrumentSync(code, fullPath);
    const { coverageData } = readInitialCoverage(instrumentedCode);
    return {
      [fullPath]: coverageData
    };
  }

• Merge each file's empty coverage data into a coverage map and supply that map to the routine c8 currently executes in getCoverageMapFromAllCoverageFiles

Does this seem like a reasonable approach? I have a local prototype of this code that on the surface seems to do what I want. I get a coverage report with my unloaded local files from cwd set to 0 across the board.

But, I was thinking perhaps there was a better/more efficient way to generate the empty coverage report?

Or is there perhaps generally a better way?

[bcoe]

@j03m @coreyfarrell I had a thought about this, and it might be worth considering for nyc too ... should files that are never loaded by a separate section of the report, rather than us trying to guess as to how many lines weren't covered?

[coreyfarrell]

I'm not sure. I worry about significantly expanding the options of nyc report to include --all:

• test-exclude options: include, exclude, extension, cwd, exclude-node-modules
• require: potentially needed to properly instrument
• parser-plugins: needed for code that contains non-standard or future syntax
• sourcemap options: unsure what would be needed here
• ignore-class-methods: --all needs to honor the same options as the normal instrumentation

This is what I can think of now, potentially other stuff involved but --all is actually quite complex and the results depend on many configuration options.

[j03m]

I think it would be nice to highlight and visually call out a file in a report if it was not loaded. Right now in the fork I'm experimenting with (and in nyc's --all if I'm not mistaken) unloaded files are simply listed as though completely uncovered. There is no way to tell at initial glance that a file is completely uncovered because it was never loaded, which is arguably information we have at the time the report is generated. It would be nice to call that out in some way.

I don't think it should be so separate however as to not factor into the aggregate total coverage numbers. I'm still very interested in knowing my project's or a folder's total coverage %. If files are loaded or unloaded, I only care so far as my investigation into whether or not I've missed some critical path in our testing and why.

That said, still being fairly new to this code base, I worry a bit about possibly needing to change all the reports in istanbul-reports/lib to support this sort of feature? But I've barely scratched the surface of that code at this point so its mostly FUD on my part.

[coreyfarrell]

There is no way to tell at initial glance that a file is completely uncovered because it was never loaded

This is actually never true for CJS files. Example:

module.exports = undefined;

module.exports = undefined is a statement that gets counted coverage if the file is loaded. Unless a CJS file is completely empty (not even any exports) it is impossible to load it get 0% coverage. In an edge case this is not true of ESM:

export default ['just', 'export', 'static', 'data'];

Since export is syntax and not a "statement" this is not viewed as a line-of-code, so this ESM example actually has 0 statements, 0 functions, 0 branches and 0 lines of code. We probably need to add code to the instrumenter to count import and export statements as statements. Testing such code with nyc --all=true --require @babel/register with babel configured to transpiled to CJS and instrument will produce missed coverage.

Ultimately I would argue that 0% coverage is 0% coverage, regardless of the file being loaded or not it shows that your tests need work. So TBH I'm missing the benefit of being able to list files that were never loaded.

[bcoe]

nyc doesn't need to change how it approaches --all, for c8 to support --all.

My thinking is that we shouldn't jump through hoops to try to guess how many lines of code are in files that haven't been loaded, perhaps the reporter could just list lines as unknown, and if you want to get a report that is 100% coverage, you need to have made an effort to test each of those files.

What I'd like to avoid in the c8 codebase is going down the rabbit hole of understanding any transpilation that might be happening in a codebase, or performing any transpiliation itself, the goal being to simply be a thin layer on top of v8's built in coverage...

Perhaps it's enough that we just take the line count of the file, and use this for the initial report, even though this might not be the true line count if the file is actually loaded?

[bcoe]

I'm missing the benefit of being able to list files that were never loaded.

If we list the files that were never loaded, taking into account your exclude rules and not listing these files, than it's indicative of the fact that you haven't hit 100% coverage.

We could exit with 1 if you've failed to test files not covered by an exclude rule.

[coreyfarrell]

I'm missing the benefit of being able to list files that were never loaded.

If we list the files that were never loaded, taking into account your exclude rules and not listing these files, than it's indicative of the fact that you haven't hit 100% coverage.

Sorry this comment was in the context of nyc which lists files that were never loaded as "0% covered". What I meant is I don't see the benefit for nyc to being able to list never loaded files separately from the files that were loaded when the details of what wasn't covered is reported.

For c8 I can see it making sense to just count the number of lines in files that weren't loaded and use that to alter the "% total lines" statistic, maybe even have a separate list of files that weren't loaded at all. I think counting the total number of lines in the file as not covered is far more accurate than not counting the file.

[j03m]

What I'd like to avoid in the c8 codebase is going down the rabbit hole of understanding any transpilation...

Can we get away from understanding the transpilation happening (via sourcemaps I guess)? Today we have to take that into account when generating reports for typescript, babel etc?

[bcoe]

@j03m I think we can get pretty far potentially:

1. either just counting the newlines in a file, or.
2. like you suggest, also looking for a source-map in the file.

[bcoe]

@j03m exciting update 😄 this library now works for ts-node:

#152

I'm definitely excited to address the --all issue, which is one of the remaining deltas we have.

I'd suggest making a best effort, anything is better than nothing 😄

[j03m]

Okay I will put up a draft PR sometime in the next day.

[j03m]

Draft here: #158

Apologies for the delay.

[bcoe]

@j03m nothing to apologize for! thanks for taking this work on. I'll apologize in advance that it might take me a few days to review 😆

[j03m]

No rush! Thanks again for creating this!

[bcoe]

@j03m thank you for the contribution 👍