LCOV: Wrong path in SF, containing file-uri, corrupted lcov can not be processed by coveralls.

[Uzlopak]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch istanbul-reports@3.1.4 for the project I'm working on.

When creating LCOV Information, the SF: entries have some broken path.

TN:
SF:src/file:/home/uzlopak/workspace/license-generator/src/LicenseGenerator.ts
FN:45,isPrototypeKeyword
FN:59,(anonymous_1)
FN:89,(anonymous_2)
FN:116,(anonymous_3)
FN:174,_prepareDataObject
FNF:5

After I apply my patch, it generates correct paths.

TN:
SF:src/LicenseGenerator.ts
FN:45,isPrototypeKeyword
FN:59,(anonymous_1)
FN:89,(anonymous_2)
FN:116,(anonymous_3)
FN:174,_prepareDataObject

First i thought it is because of the relative path issues. Maybe somebody can make a deep dive and figure out why "file:" is added and generating corrupt lcov files, which can not be handled by coveralls.

diff --git a/node_modules/istanbul-reports/lib/lcovonly/index.js b/node_modules/istanbul-reports/lib/lcovonly/index.js
index 0720e46..91a6805 100644
--- a/node_modules/istanbul-reports/lib/lcovonly/index.js
+++ b/node_modules/istanbul-reports/lib/lcovonly/index.js
@@ -30,7 +30,7 @@ class LcovOnlyReport extends ReportBase {
         const path = require('path');
 
         writer.println('TN:');
-        const fileName = path.relative(this.projectRoot, fc.path);
+        const fileName = path.relative(this.projectRoot, fc.path.replace(/(.*file:)/, ''));
         writer.println('SF:' + fileName);
 
         Object.values(functionMap).forEach(meta => {

This issue body was partially generated by patch-package.

[cspotcode]

This is also causing TypeStrong/ts-node#1790

The sourcemap spec says that sourceRoot / sources can be / should be file URIs. https://sourcemaps.info/spec.html

Resolving Sources

If the sources are not absolute URLs after prepending of the “sourceRoot”, the sources are resolved relative to the SourceMap (like resolving script src in a html document).

Yet istanbul does not appear to support URIs:
https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-source-maps/lib/pathutils.js

[Uzlopak]

Do you know when the fc.path gets generated? It is already broken when it is arriving in lcov reporter.

[Jason3S]

@Uzlopak,

An update to ts-node changed the inputSourceMap.file and inputSourceMap.sources to be absolute URLs.

Old Source Map

        "inputSourceMap": {
            "version": 3,
            "file": "/Users/jason/projects/rx-stream/src/index.ts",
            "sources": [
                "/Users/jason/projects/rx-stream/src/index.ts"
            ],
            "names": [],
            "mappings": ";;;;;;;;;;;;;;;;AAAA,+CAA6B;AAC7B,+CAA6B",
            "sourcesContent": [
                "export * from './rxToStream';\nexport * from './streamToRx';\n"
            ]
        },

New Source Map

        "inputSourceMap": {
            "version": 3,
            "file": "file:///Users/jason/projects/rx-stream/src/index.ts",
            "sources": [
                "file:///Users/jason/projects/rx-stream/src/index.ts"
            ],
            "names": [],
            "mappings": ";;;;;;;;;;;;;;;;AAAA,+CAA6B;AAC7B,+CAA6B",
            "sourcesContent": [
                "export * from './rxToStream';\nexport * from './streamToRx';\n"
            ]
        },

As far as I can tell, Instanbul was not designed to work with real absolute file:// URLs. This code treats them like file paths:

istanbuljs/packages/istanbul-lib-source-maps/lib/map-store.js

Lines 52 to 76 in c155900

	registerURL(transformedFilePath, sourceMapUrl) {
	const d = 'data:';
	if (
	sourceMapUrl.length > d.length &&
	sourceMapUrl.substring(0, d.length) === d
	) {
	const b64 = 'base64,';
	const pos = sourceMapUrl.indexOf(b64);
	if (pos > 0) {
	this.data[transformedFilePath] = {
	type: 'encoded',
	data: sourceMapUrl.substring(pos + b64.length)
	};
	} else {
	debug(`Unable to interpret source map URL: ${sourceMapUrl}`);
	}
	return;
	}
	const dir = path.dirname(path.resolve(transformedFilePath));
	const file = path.resolve(dir, sourceMapUrl);
	this.data[transformedFilePath] = { type: 'file', data: file };
	}

[cspotcode]

For the istanbul maintainers, I think this issue can be renamed into a bug report:
istanbul does not handle spec-compliant sourcemaps with file:// URI sources / sourceRoot

The bugfix is for istanbul to detect when sourceRoot / sources are file URIs and convert them to native paths, allowing the rest of instanbul's coverage reporting to function as expected.

[Jason3S]

@cspotcode,

Supporting absolute file URLs is a feature request, not a bug. They have correctly implemented the relative URLs and it has worked for many years. Just because a choice to fix a bug in ts-node was to use absolute file URLs instead of relative URLs should not constitute a bug in Istanbul. The bug in ts-node could have been fixed with a relative file URL.

[cspotcode]

I'm not trying to stir up trouble. However, I think it's fair to say that istanbul intends to support sourcemaps in whatever spec-compliant form they may take, since they might not be generated by ts-node. So I'm certainly not trying to place blame, but also I feel like I have a responsibility to move the ecosystem in the correct direction.

Just to make this a bit clearer, if the istanbul maintainers confirm that they agree, then I can likely send them a pull request with the fix. It's really about making the right fixes in the right places.

[Jason3S]

@cspotcode,

I agree that the ecosystem should move forwards and support absolute URLs as per the spec. I think a PR would be a good idea.

I just think this is a feature request vs a bug.

[rantoniuk]

Just for reference: https://github.com/TypeStrong/ts-node/releases/tag/v10.8.2

[bcoe]

Just to make this a bit clearer, if the istanbul maintainers confirm that they agree, then I can likely send them a pull request with the fix. It's really about making the right fixes in the right places.

@cspotcode IstanbulJS is essentially in maintenance mode. I make a best effort to keep up on security patches, bug fixes. All this to say, there's not an opinionated technical direction for the project.

We should definitely support file URLs, if the spec indicates this is an expectation 👍 will happily accept a patch for this.

[squidfunk]

@bcoe are there viable alternatives you would recommend switching to or are you just considering Istanbul feature-complete? As far as my research goes, Istanbul together with NYC is basically the only way to get code coverage up and running with Jasmine.

[bcoe]

are there viable alternatives you would recommend switching to or are you just considering Istanbul feature-complete?

For my purposes, I largely find it to be feature complete.

For personal projects I use c8, which uses parts of the Istanbul codebase (but not the instrumentation bits).

Note: community patches are still being released from this library, I'm just not doing active development work.

[cspotcode]

As far as I can tell, the PR to fix this will be small. For potential implementers:

These two functions need to understand when they've received a file URI.
https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-source-maps/lib/pathutils.js

I'm sure there are plenty of great ways to identify and manipulate file URIs. Here is one:
https://github.com/jridgewell/resolve-uri/
It's MIT licensed, so pretty sure bits can be copy-pasted with the correct license attribution, if adding a dep is for some reason undesirable.