[feat] module api for intergrators

[Toxicable]

Im integrating code coverage into Bazel: https://github.com/bazelbuild/rules_nodejs
However, rather than reimplementing the internal logic we see in this lib it would be great if there was a public facing API that could be used by downstream integrators.

For this kind of integration the main functionality needed

• gathering coverage (without child process/forking)
• producing machine readable reports (in this case we're after lcov)
• merging reports between projects that might have overlapping coverage

A code example of the api we're after might look like this

const c8 = require('c8');

c8.collectCoverage({reporters: ['text-summary', 'lcov']});
jasmine.execute();
const reports = await c8.completeCoverage();

const textSummaryReport = reports.find(r => r.type = 'text-summary');
console.log(textSummaryReport);

cons lcovReport = reports.find(r => r.type = 'lcov');
fs.writeFileSync('to/some/location', lcovReport);
// or maybe merge/process it in some other way

Some important points here:

• The lib should not do any IO
• The lib should not require environment variables to collect coverage

I'd be happy to contribute to this design or implementation if needed.
Let me know if this is something we'd like to do with this lib

[bcoe]

@Toxicable I would look at how we approach collecting coverage for Node.js:

https://github.com/nodejs/node/blob/master/Makefile#L245

You can use c8 for reporting, without it executing the underlying Node.js bin itself. You will however need to pass the NODE_V8_COVERAGE flag to Node.js, the restriction on setting environment variables feels like it would be a non-starter.

[Toxicable]

@bcoe My understanding is that you can activate code coverage collection via the inspector api.
The key strings are Profiler.startPreciseCoverage, Profiler.stopPreciseCoverage
Here's a blog post on it https://v8.dev/blog/javascript-code-coverage#for-embedders

[SimenB]

(I posted this on Slack, but I can post it here as well as it's probably a better place).

Background: I'm looking into integrating V8 coverage into Jest (jestjs/jest#7062).

---

Sample code I want to collect coverage for:

const vm = require('vm');

const script = new vm.Script(`
function thing() {
  if (Math.random() > 0.5) {
    output = 'success!';
  } else {
    output = 'failure';
  }
}

thing();
`);

const context = vm.createContext({ output: '' });

script.runInContext(context);

The catch here is that I'm just interested in coverage for script, not anything else. Filtering it out should be relatively straightforward once we've got something working though.

I played around with using the inspector API mentioned in the above blog post, and I got it to sorta work.

I get this output:

{
  "scriptId": "69",
  "url": "evalmachine.<anonymous>",
  "functions": [
    {
      "functionName": "",
      "ranges": [{ "startOffset": 0, "endOffset": 125, "count": 1 }],
      "isBlockCoverage": false
    },
    {
      "functionName": "thing",
      "ranges": [{ "startOffset": 1, "endOffset": 114, "count": 1 }],
      "isBlockCoverage": false
    }
  ]
}

If I instead set NODE_V8_COVERAGE instead of manual inspector code, I get the following

{
  "scriptId": "70",
  "url": "evalmachine.<anonymous>",
  "functions": [
    {
      "functionName": "",
      "ranges": [{ "startOffset": 0, "endOffset": 125, "count": 1 }],
      "isBlockCoverage": true
    },
    {
      "functionName": "thing",
      "ranges": [
        { "startOffset": 1, "endOffset": 114, "count": 1 },
        { "startOffset": 47, "endOffset": 77, "count": 0 }
      ],
      "isBlockCoverage": true
    }
  ]
}

Differences being that isBlockCoverage is set to true (I'm not sure what this means) and more importantly the added { "startOffset": 47, "endOffset": 77, "count": 0 } entry, which shows what code was not covered. I'm not sure why it differs.

Running with NODE_V8_COVERAGE makes no difference for what I get back from manually running the inspector, which I guess makes sense (different sessions).

It might be that it doesn't work properly for me due to the fact we use vm.Script to execute things, and it otherwise works fine?

My testing code, for completeness:

const inspector = require('inspector');
const vm = require('vm');
const { builtinModules } = require('module');
const session = new inspector.Session();
session.connect();

const script = new vm.Script(`
function thing() {
  if (Math.random() > 0.5) {
    output = 'success!';
  } else {
    output = 'failure';
  }
}

thing();
`);

const context = vm.createContext({ output: '' });

session.post('Profiler.enable', () => {
  session.post('Profiler.startPreciseCoverage', () => {
    script.runInContext(context);

    session.post('Profiler.takePreciseCoverage', (err, { result }) => {
      if (err) {
        console.error(err);
        process.exitCode = 1;

        return;
      }

      const filtered = result.filter(
        ({ url }) =>
          !builtinModules.some(builtin => builtin + '.js' === url) &&
          !url.startsWith('internal/')
      );

      console.log(JSON.stringify(filtered));

      session.post('Profiler.stopPreciseCoverage', err => {
        if (err) {
          console.error(err);
          process.exitCode = 1;
        }
      });
    });
  });
});

[bcoe]

@Toxicable Node.js has an inspector session running by default in most platforms, which NODE_V8_COVERAGE allows us to piggyback off of, and ensures that coverage is enabled very early on in the Node.js bootstrapping process.

Creating a new inspector session and negotiating the connection with Node.js is a pain, and much more error prone. I think it's much better to spawn Node.js with a flag or environment variable, then try to negotiate the inspector dance in userland (this is actually what the first pass at this library did, and it worked terribly).

[Toxicable]

@bcoe Thats interesting, thanks for the background on that.
With that in mind we might have to revise our design on this a little bit on the bazel side of things.

Just for context; one of the main reasons we don't want to do a sub process is becasue we currently override the module resolution functionality of require so nodejs can resolvem modules running with bazel's runfile layout.
So when we fork the process, the forked process dosen't have the overwriten module resolution.

However, we could add coverage support further up the chain - at the original nodejs spawn, rather than at the test runners execution, ill explore this design a bit more.

---

With that constrat in mind, how about Reporting and Merging?
We'd need reporting on each set of coverage
Then mergeing of each of those sets into a final report for the whole repostitory

Are there currently any tools for merging v8 coverage of common libs together for a final version?

[bcoe]

@Toxicable you should be able to use c8 today to merge reports 👍 you just collect the raw coverage output, and run c8 against the folder with the command report.

As for spawning subprocesses, you shouldn't need to do this. You just need to make sure that the initial process spawned has NODE_V8_COVERAGE set (you don't need to use c8, just set this in the environment of the top-level Node.js process being spawned).

c8 doesn't really do that much:

• it allows you to spawn a Node.js application with NODE_V8_COVERAGE set (something you can opt to do outside of c8, like we do in the Node.js codebase).
• it knows how to convert from the raw V8 inspector format to a format more easily consumed by Istanbul.
• It knows how to pipe this converted coverage output into Istanbul's libraries, creating pretty reports.

Most of the heavy lifting is in Node.js itself, via the inspector session.

[bcoe]

I should add, if there is a way we can better expose this information from Node.js, happy to try to figure something out with you -- but setting NODE_V8_COVERAGE seemed like one of the easier interfaces to me (this in turn turns on coverage in the inspector session running in Node.js).

[Toxicable]

Just took a shot at implementing NODE_V8_COVERAGE but ran into the issue that I realised was the reason for wanting to use the inspector api way back when I first looked at this.
Since Nodejs (or maybe v8) only writes the coverage file once the process exits, it dosen't have access to the report itself.
Is there a way to tell Nodejs that we're done and it should write it's file?

Im not sure adding another program for handling coverage after the primary one is a good design.

If you're curious this is the work so far on this: bazel-contrib/rules_nodejs#874

[SimenB]

That's also needed for Jest's use case - we support coverage in watch mode. I also need to generate the reports in-process since the location of sourcemaps are in a temp cache somewhere (which we manage in memory)

[bcoe]

@Toxicable @SimenB might be worth moving this feature request to Node.js? could imagine this being possible with a signal, not sure what signal.

[Toxicable]

@bcoe That sounds reasonable, do you have permissions to move this issue?
Or shall I make a new one referencing this?

[Toxicable]

Made a new one here: nodejs/node#28283

[SimenB]

I opened up istanbuljs/v8-to-istanbul#33. By using that, and copying some istanbul-lib-* code from Report.js in this repo, I was able to make it work in Jest 🎉

The part I copied from c8:

const map = libCoverage.createCoverageMap({});

v8CoverageResult.forEach(res => map.merge(res));

const context = libReport.createContext({
  dir: globalConfig.coverageDirectory,
});

const tree = libReport.summarizers.pkg(map);

tree.visit(reports.create('html'), context);

Not too much. So I don't think we necessarily need to put any more code in c8 - making v8-to-istanbul support not talking to the FS seems to cover it?

---

The inspector API abstraction I put together: https://github.com/SimenB/jest/blob/917efc3398577c205f33c1c2f9a1aeabfaad6f7d/packages/jest-coverage/src/index.ts

Usage:

const v8CoverageInstrumenter = new CoverageInstrumenter();

await v8CoverageInstrumenter.startInstrumenting();

// do your work that you want instrumented here

const v8CoverageResult = await v8CoverageInstrumenter.stopInstrumenting();

Note that this does not at all have to live in Jest, it was just easiest for me when doing this work to have it there. It might live in the istanbul org, maybe?

---

Results in this:

(this is just require('typescript'))

[bcoe]

@SimenB cool, let's move this conversation to v8-to-istanbul 👍