gotwarlost / istanbul

Yet another JS code coverage tool that computes statement, line, function and branch coverage with module loader hooks to transparently add coverage when running tests. Supports all JS coverage use cases including unit tests, server side functional tests and browser tests. Built for scale.
Other
8.7k stars 788 forks source link

Instrumenter adds a `\r` to strings in Windows #114

Closed juandopazo closed 10 years ago

juandopazo commented 11 years ago

Hi,

Istanbul is introducing a \r string in the resulting coverage files on Windows. This is making YUI users on Windows introduce noise during builds.

Steps to reproduce this:

source-file.js

function foo() {
    console.log('hello world');
}

Intrumenter/build-file.js

var istanbul = require('istanbul');
var fs = require('fs');

var inst = new istanbul.Instrumenter({
    embedSource: true
});
inst.instrument(fs.readFileSync('source-file.js', 'utf8'), function (err, data) {
    console.log(data);
});

windows-result.js

if (typeof __coverage__ === 'undefined') { __coverage__ = {}; }
if (!__coverage__['1383323168552.js']) {
   __coverage__['1383323168552.js'] = {"path":"1383323168552.js","s":{"1":0,"2":0},"b":{},"f":{"1":0},"fnMap":{"1":{"name":"foo","line":1,"loc":{"start":{"line":1,"column":-15},"end":{"line":1,"column":15}}}},"statementMap":{"1":{"start":{"line":1,"column":-15},"end":{"line":3,"column":1}},"2":{"start":{"line":2,"column":1},"end":{"line":2,"column":28}}},"branchMap":{},"code":["(function () { function foo() {\r","\tconsole.log('hello world');\r","}","}());"]};
}
var __cov_HBZ_dWeQ59RjvazWS_$nBA = __coverage__['1383323168552.js'];
__cov_HBZ_dWeQ59RjvazWS_$nBA.s['1']++;function foo(){__cov_HBZ_dWeQ59RjvazWS_$nBA.f['1']++;__cov_HBZ_dWeQ59RjvazWS_$nBA.s['2']++;console.log('hello world');}

/cc @davglass @reid

gotwarlost commented 11 years ago

what exactly is the problem with the noise? I mean what sort of messages do you see

juandopazo commented 11 years ago

Notmally it wouldn't be a problem, but our team has developers using both OSX and Windows so every time a Windows developers builds YUI it adds a lot of noise to the diffs.

The fix seems to be just splitting lines with /\r?\n/ in the instrumenter part that creates the lines array. I'll send a pull request once I'm sure that's the correct fix.

Juan

juandopazo commented 10 years ago

Thank you!!!

gotwarlost commented 10 years ago

Available in v0.1.46

ezequiel commented 10 years ago

@gotwarlost,

It appears this fix didn't actually fix the problem. I still see a lot of \r noise in the coverage files. I'm using yogi to help generate these coverage files.

However, I reverted back to istanbul v0.1.37, applied the fix seen here: https://github.com/gotwarlost/istanbul/blob/bec0e4b2cce51c75043e6a95c0151dc253894636/lib/instrumenter.js#L497 and the problem magically fixed itself.

It's possible yogi is reintroducing the new \rs for v0.1.46, but that doesn't seem to be the case for v0.1.37.