No server-side coverage since Meteor 1.6 recorded

[SimonSimCity]

I have a project, where I use this plugin here to generate code-coverage. Today I tried to update to Meteor 1.6, which worked perfect and all my tests ran through, but the code-coverage only showed the lines as covered which are included in the code, run on the client (browser), but not those lines run on the server.

I called it by the following command:

spacejam-mocha test --settings settings-test.json --coverage 'out_json_report out_coverage'

I use lmieulet:meteor-coverage in version 1.1.4 and just switched Meteor from 1.5.2.1 to 1.6-rc.17.

[SimonSimCity]

Could confirm it by calling:

COVERAGE_VERBOSE=1 COVERAGE=1 COVERAGE_APP_FOLDER=$(pwd)/ meteor

If the project is running on meteor 1.5.2.1 I see a lot of [Ignored][exclude.server] lines at the beginning for all the js files from the /packages/ folder. When upgrading to meteor 1.6-rc.17 I don't see these ignores anymore.

[serut]

They may have change the way they are processing server files before starting the server. You don't have any clue except that some logs are missing

[SimonSimCity]

Nope, but I'll have to dig into it if you don't have time to fix it. Any help and details about how it works is appreciated ...

All I know is, that they switched from NodeJS 4.x.x to NodeJS 8.8.0. Maybe this had an impact on it. Maybe NodeJS introduced something, which could be solved by updating the Istanbul plugin - which is the first thing I'd give a try.

[serut]

On the node side, when meteor-coverage is imported by meteor, any module loaded after that will be covered. meteor-coverage asks istanbul to catch all call to vm.runInThisContext, a (very low level) function used to require a file (jquery, react, your app,..) from the current one. It alters the function globally, so when some code in your app import 'something', it basically calls vm.runInThisContext which is a hook now meteor-coverage have been executed.

meteor-coverage execute that in the file server/services/instrumenter.js:L29-35.

I've created a function that is called everytimes istanbul received a call by node, to decide if the file should be instrumented or not. Just add a log inside the function shallInstrumentServerScript.

If you have nothing, something changed with node V8 !

To let you test what I propose you, you need to clone my library in your <app folder>/packages directory (example). Once cloned, meteor detects that the package can be retrieved in your <app folder>/packages and not in the meteor central folder where modules are stored.

Good luck ! If you want to upgrade istanbul you can look if the API did not change too.

[SimonSimCity]

It seems like meteor changed the way of loading the extensions. I've added a console.log() to your shallInstrumentServerScript function which wasn't called a single time. After searching through the changes, I discovered a method-call of vm.runInThisContext() in tools/static-assets/server/boot.js and added some logging there.

There were quite some changes on this file, most notably one, introduced with the node-debugger. If you follow closely, you'll see why.

In the new code, the function, returned by vm.runInThisContext() is passed to an array and executed after all the modules are loaded. This (in return) has the effect, that meteor-coverage - as it is a normal meteor module - only is initialized after every module is loaded (using vm.runInThisContext()).

But this is not the only thing that needs to be changed ... The changes in this commit also need to be reverted or rewritten, because the package istanbul-api doesn't support using vm.Script.runInThisContext() yet.

I can confirm that it works after changing these two parts of boot.js in Meteor. No changes needed on his plugin.

I've registered it at the meteor repository. Let's see what we need to go through here to get it finaly done ...

[serut]

Can you try to replace

    var func = require('vm').runInThisContext(wrapped, {
      filename: scriptPath,
      displayErrors: true
    });

instead of

    var script = new (require('vm').Script)(wrapped, {
      filename: scriptPath,
      displayErrors: true
    });

    var func = script.runInThisContext();

I think too that this is related to meteor/meteor@f244a5b

[SimonSimCity]

Doesn't work ... I tried it. And the main reason for that (I guess) is the implementation of istanbul-api (https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-hook/lib/hook.js#L137) which expects the second argument to be the filename.

[serut]

Nice catch. There is an open PR https://github.com/istanbuljs/istanbuljs/pull/99/files that wants to fix that issue. In that case the only error on the Meteor side is just to use require('vm').Script.runInThisContext()

[SimonSimCity]

But as I described here, this seems just to be half of what's needed to be done in order to get it working: See my second point on meteor/meteor#9281

[serut]

No, I tried again and I don't need any modification of the meteor boot.js file.

I rollback my file to the original one, added a log console.error("ME", scriptPath) and the log is fine :

> meteor test --settings settings.coverage.json --driver-package practicalmeteor:mocha

[[[[[ Tests ]]]]]

=> Started proxy.
=> Started MongoDB.
W20171112-13:43:48.018(1)? (STDERR) ME packages\meteor.js
W20171112-13:43:48.452(1)? (STDERR) ME packages\meteor-base.js
W20171112-13:43:48.453(1)? (STDERR) ME packages\mobile-experience.js
W20171112-13:43:48.453(1)? (STDERR) ME packages\npm-mongo.js
W20171112-13:43:48.455(1)? (STDERR) ME packages\ecmascript-runtime.js
W20171112-13:43:48.455(1)? (STDERR) ME packages\modules-runtime.js
W20171112-13:43:48.456(1)? (STDERR) ME packages\modules.js
W20171112-13:43:48.457(1)? (STDERR) ME packages\ecmascript-runtime-server.js
W20171112-13:43:48.458(1)? (STDERR) ME packages\babel-compiler.js
W20171112-13:43:48.459(1)? (STDERR) ME <windows path>\AppData\Local\Temp\meteor-test-runpu60il.fh2g\.meteor\local\build\programs\server\packages\ecmascript.js

[SimonSimCity]

@serut You will get this log for a couple of files, true, but as you see here, none of them actually is from your modules folder. It's only about the files in packages/* and node_modules/*.

After applying my fixes again (in addition to the PR which I had all the time), I found this one:

W20171129-10:36:50.376(4)? (STDERR) Module load hook: transform [/private/var/folders/ky/vp_prm4178x_yhbb7nydy5dh0000gn/T/meteor-test-runerfyq.noskyb/.meteor/local/build/programs/server/app/app.js]
W20171129-10:36:50.425(4)? (STDERR) Transformation error for /private/var/folders/ky/vp_prm4178x_yhbb7nydy5dh0000gn/T/meteor-test-runerfyq.noskyb/.meteor/local/build/programs/server/app/app.js ; return original code
W20171129-10:36:50.425(4)? (STDERR) Data must be a string or a buffer
W20171129-10:36:50.426(4)? (STDERR) TypeError: Data must be a string or a buffer
W20171129-10:36:50.426(4)? (STDERR)     at Hash.update (crypto.js:99:16)
W20171129-10:36:50.426(4)? (STDERR)     at genVar (/Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-instrument/dist/visitor.js:33:10)
W20171129-10:36:50.427(4)? (STDERR)     at new VisitState (/Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-instrument/dist/visitor.js:44:24)
W20171129-10:36:50.427(4)? (STDERR)     at programVisitor (/Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-instrument/dist/visitor.js:550:22)
W20171129-10:36:50.427(4)? (STDERR)     at Instrumenter.instrumentSync (/Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-instrument/dist/instrumenter.js:126:44)
W20171129-10:36:50.427(4)? (STDERR)     at /Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-hook/lib/hook.js:29:31
W20171129-10:36:50.428(4)? (STDERR)     at Object.vm.runInThisContext (/Volumes/WebDocuments/my-project/.meteor/local/isopacks/lmieulet_meteor-coverage/npm/node_modules/istanbul-lib-hook/lib/hook.js:146:19)
W20171129-10:36:50.428(4)? (STDERR)     at /private/var/folders/ky/vp_prm4178x_yhbb7nydy5dh0000gn/T/meteor-test-runerfyq.noskyb/.meteor/local/build/programs/server/boot.js:392:30
W20171129-10:36:50.428(4)? (STDERR)     at Array.forEach (<anonymous>)
W20171129-10:36:50.428(4)? (STDERR)     at Function._.each._.forEach (/Users/simon/.meteor/packages/meteor-tool/.1.6.0.1vkwqrb++os.osx.x86_64+web.browser+web.cordova/mt-os.osx.x86_64/dev_bundle/server-lib/node_modules/underscore/underscore.js:79:11)

This is after I had the PR in place and the changes I noted above are applied.

[serut]

If I'm right, /private/var/folders/ky/vp_prm4178x_yhbb7nydy5dh0000gn/T/meteor-test-runerfyq.noskyb/.meteor/local/build/programs/server/app/app.js is your entire server app code, and when it tries to instrument it, it uses the current implementation of istanbuljs, which fails to retrieve the content of the file.

On the other hand, if there was an issue with Meteor and new (require('vm').Script), istanbuljs would not be called at all, no ?

[SimonSimCity]

@serut As I wrote: When I start off with a clean installation and apply the changes from the PR, IstanbulJS is never called.

Only after again applying the changes I noted in meteor/meteor#9281, I got the exception I noted before, which comes from an incompatibility from this extension towards the changes of the library IstanbulJS, if I remember it correctly.

I'll run another test now the days to come in order to get it right again, including the latest changes on the IstanbulJS library.

EDIT: Sorry, I had a pretty messed up environment today. Details will follow soon.