source map support in coverage reports

[Toxicable]

🚀 feature request

Relevant Rules

jasmine_node_test

Description

We should still use v8-coverage to collect coverage reports without having to fork the process.
However, c8 and transitively v8-to-istanbul provide better reporting due to having source map support and more up to date istanbul deps.

[Toxicable]

This however is blocked on #823
Since the source map lib needs to resolve the souce map back to the source file from the executed file.

Also blocked on and istanbuljs/v8-to-istanbul#31

[Toxicable]

Aside from the blocking issues it's minor changes needed on our side so just going to document that here for now:

        const Report = require('c8/lib/report');
        fs.mkdirpSync(path.join(covDir, 'tmp'));
        // we have to write it in a top level result object since thats what the lib expecets
        fs.writeFileSync(path.join(covDir, 'tmp', 'coverage-final.json'), JSON.stringify({result: sourceCoverge}));

        const report = Report({
          reportsDirectory: covDir,
          tempDirectory: path.join(covDir, 'tmp'),
          reporter: ['text', 'text-summary', 'html'],
          exclude: ['**/node_modules/**'],
        });

[Toxicable]

I think the best way to handle this would be to add a coverage flag to nodejs_binary which would then internally set the NODE_V8_COVERAGE to a dir where the collection will be written.
We need to do this at a nodejs_binary level so that we don't have to use the inspector api and so that we don't have to use a fork later on.

We'll still need to handle the reporting at the end of jasmine_node_test.

However, if we switched to c8 for reporting and transitively v8-to-istanbul we'd need the source maps situation to be fixed, since they are unable to resolve our source files in the current setup.
As mentioned above.
Which is the same issue here: #823

[thesayyn]

Hello @Toxicable is there any update on this

[Toxicable]

@thesayyn Yeah so I know what needs to be done to make this change
However due to #823 the result will be broken since our source maps don't map back to sources correctly and after talking with @alexeagle I don't know if there is actually a solution to that problem.

So untill we figure out source maps properly, this is blocked

[sjoerdvisscher]

Hi, @Toxicable, the issues you point to here are now closed. Does this mean this issue has a chance to progress?

[sjoerdvisscher]

I have working TS coverage now with two patches:

1. feat: add support for 1:1 sourcesContent istanbuljs/v8-to-istanbul#115 (This prevents c8 from reading from the invalid source location) Add "resolutions": { "v8-to-istanbul": "6.0.0" } to package.json to apply this.
2. patching lcov_merger-js to replace SF:../../../ with SF: in lcov.info

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[github-actions]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"