search

felixge / node-sandboxed-module

A sandboxed node.js module loader that lets you inject dependencies into your modules.
MIT License
342 stars 42 forks source link

Sandboxed module doesn't play nice with code coverage tools like istanbul #25

Closed searls closed 10 years ago

searls commented 10 years ago

I'm using sandboxed-module to practice TDD with isolated unit tests and I'm absolutely loving it—fantastic contribution, all. Thank you! :sparkles:

One thing I'm playing with this evening is integrating istanbul for code coverage and I noticed something fun: stuff required via sandboxed module is missed by istanbul's module load hook that it uses for ad hoc instrumentation of JS. As a result, the subject code I sandbox-require from my unit tests is never required from the coverage tool's perspective. Whoops!

I'm going to work on this through more nuanced usage of istanbul, but in case anyone here has an interest or has seen something like this before I figured it couldn't hurt to open a thread and ask you all.

Cheers!

searls commented 10 years ago

Glad to report that I got this monkey patch working completely well: https://gist.github.com/searls/7001427

Not sure whether or how it informs this project. Feel free to close it.

felixge commented 10 years ago

Not sure whether or how it informs this project. Feel free to close it.

@searls depends. Would you like your patch to become part of this project?

searls commented 10 years ago

My take is this:

  1. This experience suggests that some interceptor scheme to wrap/instrument/compile the file source would be useful. For example, right now CoffeeScript is done ad-hoc inside that getCompileInfo function I overwrote. It might be better for node-sandboxed-module to ship with one or two interceptors that could run against that source before passing wrapping it.
  2. In particular, the istanbul/coverage hack that I did might be useful but could also expand the dependencies and scope of the library beyond what you're interested in. That's obviously up to you.

I'll close the issue, just food for thought in how you want to manage the project. If you're really excited by this idea, I'd be willing to consider submitting a PR.

felixge commented 10 years ago

I'm not writing a lot of node.js code these days, so I don't feel strongly about this. But if you / somebody else does, I'd be happy to merge a pull request / give you push access to the repo.

searls commented 10 years ago

Right on, thank you. I just put out a feeler to my @testdouble friends in case anyone's interested in pairing with me on this.

On Wed, Oct 16, 2013 at 11:12 AM, Felix Geisendörfer < notifications@github.com> wrote:

I'm not writing a lot of node.js code these days, so I don't feel strongly about this. But if you / somebody else does, I'd be happy to merge a pull request / give you push access to the repo.

— Reply to this email directly or view it on GitHubhttps://github.com/felixge/node-sandboxed-module/issues/25#issuecomment-26426898 .