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 786 forks source link

FunctionDeclarations default to a cover count of 1 #102

Open derek opened 11 years ago

derek commented 11 years ago

The following code will produce a coverage object that contains a line of coverage despite having never been executed.

var istanbul = require('istanbul'),
    instrumenter = new istanbul.Instrumenter();

instrumenter.instrumentSync('function foo() {};');
console.log(instrumenter.lastFileCoverage());

It appears this "bug" (debatable) was introduced in #67.

The reason why I feel this is a bug, and why I'm using the undocumented(?) instrumenter#lastFileCoverage method, is because I am attempting to generate a coverage object consumable by collector#add for code that I have no intention of actually executing. This is solely for reporting purposes, where I'd like files that are not executed by my unit tests to appear in the HTML report as having 0/0/0/0 coverage. But, if this capability is not supported, then I guess this wouldn't be considered a bug.

FWIW, ideally I'd like to pass a Collector or Reporter an array of paths I'd like to include in the report with zero coverage, but that would simply be sugar around existing capabilities.

gotwarlost commented 11 years ago

Of course, the correct fix is to not consider declarations as statements at all but it will have the effect of lowering peoples' existing coverage numbers and is not backwards compatible.

Thinking on how best to deal with this... Ideas welcome.

derek commented 11 years ago

Considering both approaches seem valid, but at the same time conflict, perhaps this is a scenario where an additional config option to Instrumenter is appropriate?

In any case, this was pretty trivial to manually set all statement counts to zero, so it's not a big deal for me.

instrumenter.instrumentSync(fs.readFileSync(path, 'utf-8'), path);
uncovered[path] = instrumenter.lastFileCoverage();
for (i in uncovered[path].s) {
    uncovered[path].s[i] = 0;
}

So perhaps address this only if others request the capability as well.