Skip to content

Commit 9db5dee

Browse files
committedNov 18, 2018
Fix coverage with sourcemaps inlined via Transform API.
Fix issue #886 "--coverage-all fails to find any files on Windows". Fix tests that rely on path strings on Windows.
1 parent 246d25d commit 9db5dee

File tree

6 files changed

+113
-14
lines changed

6 files changed

+113
-14
lines changed
 

‎lib/coverage.js

+28-8
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,7 @@ internals.instrument = function (filename) {
297297

298298
// Store original source
299299

300-
const transformedFile = content.replace(/\/\/\#(.*)\r?\n?$/, '');
301-
internals.sources[filename] = transformedFile.replace(/(\r\n|\n|\r)/gm, '\n').split('\n');
300+
internals.sources[filename] = content.replace(/(\r\n|\n|\r)/gm, '\n').split('\n');
302301

303302
// Setup global report container
304303
// $lab:coverage:off$
@@ -368,10 +367,6 @@ internals.instrument = function (filename) {
368367

369368
return commented;
370369
}).reduce((a, b) => a + b, 0);
371-
372-
if (transformedFile !== content) {
373-
record.sloc++;
374-
}
375370
}
376371

377372
return chunks.join('');
@@ -453,7 +448,7 @@ exports.analyze = async function (options) {
453448
}
454449

455450
for (let i = 0; i < files.length; ++i) {
456-
const filename = files[i];
451+
const filename = files[i].replace(/\\/g, '/');
457452
if (pattern.test(filename)) {
Has a conversation. Original line has a conversation.
458453
if (!report.files[filename]) {
459454
internals.instrument(filename);
@@ -551,7 +546,32 @@ internals.file = async function (filename, data, options) {
551546
};
552547

553548
// Use sourcemap consumer rather than SourceMapSupport.mapSourcePosition itself which perform path transformations
554-
const sourcemap = options.sourcemaps ? SourceMapSupport.retrieveSourceMap(ret.filename) : null;
549+
let sourcemap = null;
550+
if (options.sourcemaps) {
551+
sourcemap = SourceMapSupport.retrieveSourceMap(ret.filename);
552+
if (!sourcemap) {
553+
const smre = /\/\/\#.*data:application\/json[^,]+base64,.*\r?\n?$/;
554+
let sourceIndex = data.source.length - 1;
555+
while (sourceIndex >= 0 && !smre.test(data.source[sourceIndex])) {
556+
sourceIndex--;
557+
}
558+
559+
if (sourceIndex >= 0) {
560+
const re = /(?:\/\/[@#][ \t]+sourceMappingURL=([^\s'"]+?)[ \t]*$)|(?:\/\*[@#][ \t]+sourceMappingURL=([^\*]+?)[ \t]*(?:\*\/)[ \t]*$)/mg;
561+
let lastMatch;
562+
let match;
563+
while (match = re.exec(data.source[sourceIndex])) {
564+
lastMatch = match;
565+
}
566+
567+
sourcemap = {
568+
url: lastMatch[1],
569+
map: Buffer.from(lastMatch[1].slice(lastMatch[1].indexOf(',') + 1), 'base64').toString()
570+
};
571+
}
572+
}
573+
}
574+
Has a conversation. Original line has a conversation.
555575
const smc = sourcemap ? await new SourceMapConsumer(sourcemap.map) : null;
556576

557577
// Process each line of code

‎test/cli.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ describe('CLI', () => {
103103
const result = await RunCli(['test/cli_error/parse.js']);
104104
expect(result.code).to.equal(1);
105105
expect(result.errorOutput).to.contain('Error requiring file');
106-
expect(result.errorOutput).to.contain('cli_error/parse_invalid.js:5');
106+
expect(result.errorOutput).to.contain(Path.normalize('cli_error/parse_invalid.js') + ':5');
107107
expect(result.errorOutput).to.not.contain('UnhandledPromiseRejectionWarning');
108108
});
109109

‎test/coverage.js

+77-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Load modules
44

55
const Fs = require('fs');
6+
const Os = require('os');
67
const Path = require('path');
78
const Module = require('module');
89
const Code = require('code');
@@ -12,7 +13,21 @@ const Lab = require('../');
1213

1314
// Declare internals
1415

15-
const internals = {};
16+
const internals = {
17+
transform: [
18+
{
19+
ext: '.inl', transform: function (content, filename) {
20+
21+
if (Buffer.isBuffer(content)) {
22+
content = content.toString();
23+
}
24+
25+
return content.concat(Os.EOL).concat('//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIndoaWxlLmpzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLGFBRUE7QUFHQTtBQUVBLEtBQU0sV0FBWSxFQUFsQixDQUdBLFFBQVEsTUFBUixDQUFpQixTQUFVLEtBQVYsQ0FBaUIsQ0FFOUIsTUFBUSxLQUFSLENBQWdCLENBQ1osTUFBUSxLQUFSLENBQ0gsQ0FFRCxNQUFPLE1BQVAsQ0FDSCxDQVBEIiwiZmlsZSI6InNvdXJjZW1hcHMtaW5saW5lLmpzIiwic291cmNlc0NvbnRlbnQiOlsiJ3VzZSBzdHJpY3QnO1xuXG4vLyBMb2FkIG1vZHVsZXNcblxuXG4vLyBEZWNsYXJlIGludGVybmFsc1xuXG5jb25zdCBpbnRlcm5hbHMgPSB7fTtcblxuXG5leHBvcnRzLm1ldGhvZCA9IGZ1bmN0aW9uICh2YWx1ZSkge1xuXG4gICAgd2hpbGUgKCB2YWx1ZSApIHtcbiAgICAgICAgdmFsdWUgPSBmYWxzZTtcbiAgICB9XG5cbiAgICByZXR1cm4gdmFsdWU7XG59O1xuIl19').concat(Os.EOL);
26+
}
27+
},
28+
{ ext: '.js', transform: null }
29+
]
30+
};
1631

1732

1833
// Test shortcuts
@@ -503,3 +518,64 @@ describe('Coverage', () => {
503518
});
504519
});
505520
});
521+
522+
describe('Coverage via Transform API', () => {
523+
524+
lab.before(() => {
525+
526+
internals.js = require.extensions['.js'];
527+
internals.inl = require.extensions['.inl'];
528+
529+
Lab.coverage.instrument({ coveragePath: Path.join(__dirname, 'coverage'), coverageExclude: 'exclude', transform: internals.transform });
530+
});
531+
532+
lab.after(() => {
533+
534+
require.extensions['.js'] = internals.js;
535+
require.extensions['.inl'] = internals.inl;
536+
});
537+
538+
it('identifies lines with partial coverage when having inline sourcemap', async () => {
539+
540+
const Test = require('./coverage/sourcemaps-transformed');
541+
542+
Test.method(false);
543+
544+
const cov = await Lab.coverage.analyze({ coveragePath: Path.join(__dirname, 'coverage/sourcemaps-transformed'), sourcemaps: true });
545+
546+
const source = cov.files[0].source;
547+
const missedLines = [];
548+
const missedChunks = [];
549+
Object.keys(source).forEach((lineNumber) => {
550+
551+
const line = source[lineNumber];
552+
if (line.miss) {
553+
missedLines.push({
554+
filename: line.originalFilename,
555+
lineNumber,
556+
originalLineNumber: line.originalLine
557+
});
558+
if (line.chunks) {
559+
line.chunks.forEach((chunk) => {
560+
561+
if (chunk.miss) {
562+
missedChunks.push({
563+
filename: chunk.originalFilename,
564+
lineNumber,
565+
originalLineNumber: chunk.originalLine,
566+
originalColumn: chunk.originalColumn
567+
});
568+
}
569+
});
570+
}
571+
}
572+
});
573+
574+
expect(missedLines).to.include([
575+
{ filename: 'while.js', lineNumber: '3', originalLineNumber: 11 }
576+
]);
577+
expect(missedChunks).to.include([
578+
{ filename: 'while.js', lineNumber: '3', originalLineNumber: 13, originalColumn: 12 }
579+
]);
580+
});
581+
});
+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
'use strict';// Load modules
2+
// Declare internals
3+
const internals={};exports.method=function(value){while(value){value=false;}return value;};

‎test/runner.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ describe('Runner', () => {
201201
const notebook = await Lab.execute(script, {}, null);
202202
expect(notebook.tests).to.have.length(1);
203203
expect(notebook.failures).to.equal(1);
204-
expect(notebook.tests[0].err.stack).to.contain('test/runner.js');
204+
expect(notebook.tests[0].err.stack).to.contain(Path.normalize('test/runner.js'));
205205
});
206206

207207
it('should fail test that returns a rejected promise', async () => {

‎test/transform.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ describe('Transform', () => {
8282

8383
require('./transform/basic-transform'); // prime the cache
8484

85-
const rel = Transform.retrieveFile('test/transform/basic-transform.new');
85+
const rel = Transform.retrieveFile(Path.normalize('test/transform/basic-transform.new'));
8686
expect(rel).to.not.contain('!NOCOMPILE!');
8787

88-
const abs = Transform.retrieveFile(process.cwd() + '/test/transform/basic-transform.new');
88+
const abs = Transform.retrieveFile(Path.normalize(process.cwd() + '/test/transform/basic-transform.new'));
8989
expect(abs).to.not.contain('!NOCOMPILE!');
9090
});
9191
});
@@ -112,7 +112,7 @@ describe('Transform.install', () => {
112112

113113
const Test = require('./transform/sourcemaps');
114114

115-
expect(Test.method(false)).to.equal(false);
115+
expect(Test.method(true)).to.equal(false);
116116

117117
const Test2 = require('./transform/exclude/transform-basic');
118118

1 commit comments

Comments
 (1)

Invader444 commented on Nov 18, 2018

@Invader444
ContributorAuthor

Even with these changes, two tests still fail on Windows! CLI multiple reporter tests continue to fail when Hoek.clone is called on the options object, which tries to assign to 'bytesWritten' on a Pipe object. I believe however that that is a bug in Hoek, and so haven't addressed it here.

Please sign in to comment.