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

Loading Files Dynamically and --hook-run-in-context documentation? #352

Open jtibble opened 9 years ago

jtibble commented 9 years ago

I'm loading JS in my Mocha test at runtime using fs and vm.runInThisContext:

fs.readFile(path, {encoding: 'utf8'}, function(error, data){})
....
vm.runInThisContext( code );

but when I run istanbul like so

$ istanbul cover --hook-run-in-context node_modules/mocha/bin/_mocha -- --recursive -R spec
...
  fake test
    √ should pass

  3 passing (28ms)

No coverage information was collected, exit without writing coverage information

Is there any documentation for how to use the --hook-run-in-context command to get code-coverage for these files?

(Windows 7 with Git Bash)

$ node -v && npm -v
v0.12.2
2.7.4
$ npm list -g mocha
C:\Users\212309975\AppData\Roaming\npm
└── mocha@2.2.4
davglass commented 9 years ago

This should cover the code that is passed into the runInThisContext method. Do you have a repro case that I can look at?

tswaters commented 8 years ago

I just encountered this issue

Based on the node api docs, I was trying the following.... While it worked when running the tests, istanbul was not picking it up.

var fileName = './path/to/file.js';
vm.runInThisContext(fs.readFileSync(fileName), {filename: path.resolve(fileName)});

So I dug into the istanbul code a bit -- hookRunInThisContext leaves a few things to be desired :

The second bullet is concerning because no where in the node docs does it say to pass a string as the second parameter to runInThisContext - it doesn't die when you do which is I suppose a silver lining... it should accept an object hash of which filename can be included.

I had to rework the above example to the following to get istanbul to recognize it:

var fileName = './path/to/file.js';
vm.runInThisContext(fs.readFileSycn(fileName).toString(), path.resolve(fileName))

I would think within the hook itself, two changes are required:

tswaters commented 8 years ago

Here we go --

diff --git a/lib/hook.js b/lib/hook.js
index 4ec9dc5..44ecf18 100644
--- a/lib/hook.js
+++ b/lib/hook.js
@@ -169,9 +169,12 @@ function unhookCreateScript() {
 function hookRunInThisContext(matcher, transformer, opts) {
     opts = opts || {};
     var fn = transformFn(matcher, transformer, opts.verbose);
-    vm.runInThisContext = function (code, file) {
-        var ret = fn(code, file);
-        return originalRunInThisContext(ret.code, file);
+    vm.runInThisContext = function (code, opts) {
+        if (!opts || !opts.filename) {
+          console.error('vm.runInThisContext called without opts.filename')
+        }
+        var ret = fn(code.toString(), (opts || {}).filename);
+        return originalRunInThisContext(ret.code, opts);
     };
 }

I was going to send a PR, but it seems many of the tests after checking out master are failing on Windows -- I'm unsure as to what unrelated things might fail as part of this.

jtibble commented 8 years ago

Great work, @tswaters! Since creating this issue I've changed jobs and now rarely work with NodeJS, but when I get back into it I'll see if I can get this working again with your diff.

Cheers, John