Use istanbul-lib-hook to wrap require and support vm hooks

[gotwarlost]

Interface changes

• Add --hook-run-in-context (default: true) to the nyc command line
• Add --temp-directory <dir> to the report command line. This is not a full implementation of a customizable tmpdir but only the bare minimum required to support self-coverage reporting for nyc. The full implementation is described in Add the ability to specify a custom tempDirectory #171

Misc changes

• Remove istanbul dev dependency in favor of APIs that are already being used, so that we don't accidentally use code from the old module any more. (I think the text reporter was being pulled in from the old lib).
• Coverage reporting is now done by nyc itself at the end of tests and this necessitates the custom temp-directory option for reporting.

Implementation

• Use istanbul-lib-hook to wrap require and hook vm.runInThisContext
• Add --hook-run-in-context option and propagate to subprocesses using the NYC_HOOK_RUN_IN_CONTEXT environment var
• Add a test that will succeed only if runInThisContext is successfully hooked.
• Replace istanbul API use with istanbul-lib-instrument and command line use with nyc itself.

[gotwarlost]

Some questions:

• I added requirejs as a dev dependency although it is actually a dependency of the test project. Is this correct or should we be doing an npm install from the test project directory before running its test?
• there is no equivalent test for these hooks apart from the CLI test. Is this needed?

Link main issue for cross-ref: #292

[coveralls]

Changes Unknown when pulling d28153c on gotwarlost:libhook into * on istanbuljs:master*.

[gotwarlost]

No idea why the appveyor build is failing. Need help.

[bcoe]

@gotwarlost AppVeyor just fails a good chunk of the time, nothing to do with this pull as long as I can tell. re-kicked it off.

[bcoe]

we've tried to be fairly batteries included so far, for instance we already instrument subprocesses right out of the gate. I might be tempted to default this to true. I'm happy with the argument name 👍

[bcoe]

This is looking great 🎉 to land this I think all we'd need to to do would be:

• add a test for vm.createScript, I don't think I saw one?
• address the couple small issues:
  • I think that enabling the new instrumentation settings by default would be smart (@jamestalmage what are your thoughts?).
  • I'd be tempted to use 'enable' rather than 'true' when passing configuration around in the env.
• finally, I'd also like to spin up my Windows VM and run this against a few projects once we're happy.

[gotwarlost]

Thanks for the detailed feedback.

• I think I'm going to remove the vm.createScript hook. The problem is that there is no place to stash the original code that was used for createScript. This is because, the instrumenter no longer supports the notion of embedding the source as part of the file coverage object. So if we ended up hooking vm.createScript the HTML report won't be able to find the original source. If someone really, really needs this we can add back the notion of embedding the source into the instrumenter and then implementing this. Even in istanbul this is a fairly exotic case and I don't believe it works correctly even today.
• I will change the setting to enable|disable instead of true|false and enable it by default

[bcoe]

@gotwarlost awesome, are you feeling like we're getting close to being able to point existing Istanbul users towards the Istanbul 1.0 APIs, and nyc? I've been having great luck as I test the new libraries against everything I throw at them, awesome work :) I wish we'd kicked this in motion sooner!

[coveralls]

Changes Unknown when pulling 03866c9 on gotwarlost:libhook into * on istanbuljs:master*.

[gotwarlost]

@bcoe I think we are getting real close, it's probably already much better than istanbul. One problem is that it is not backwards compatible and we need to go through a documentation exercise and more importantly, provide a conversion program that will take a istanbul.yaml and turn it into a nyc config file, showing warnings with unsupported features etc.

I think a big gap from istanbul.yaml is the inability to configure reporting details in nyc (e.g. set the text reporter to write to a file, for example), but those are minority use-cases.

[gotwarlost]

OK, the latest commit has all the PR comments included in it. Please take a look. I can squash commits after things look ok. Specifically, please check bundle dependency changes in package.json - I'm not quite sure abut what I did there.

[gotwarlost]

Also, I'm not sure if coveralls not being able to tell how coverage changed is a regression in the way lcov info is being fed to it (lcov now generated using nyc instead of istanbul)

[bcoe]

@gotwarlost this is feeling like a coveralls.io issue, I find there's a lot of weirdness with various services when I transfer repo ownership.