Colingo / mock

Load a module with it's require's mocked out
MIT License
13 stars 3 forks source link

browserify support #1

Open chrismatheson opened 9 years ago

chrismatheson commented 9 years ago

Not sure if this is a new problem, but after trying to use in a broswerify@4.2.3 project im seeing the following error

TypeError: 'undefined' is not a function (evaluating 'require.resolve(name)')
        at mock (/var/folders/xn/wtrn3yyn1l5_fzc274fc7x_c0000gn/T/c402bf8a7ce5437b5ec6c2b2a775f22a69fb442a.browserify:1903)
        at /var/folders/xn/wtrn3yyn1l5_fzc274fc7x_c0000gn/T/c402bf8a7ce5437b5ec6c2b2a775f22a69fb442a.browserify:72
        at /Users/chrismatheson/Code/quote-widget/node_modules/karma-jasmine/lib/boot.js:113
        at /Users/chrismatheson/Code/quote-widget/node_modules/karma-jasmine/lib/adapter.js:171
        at http://localhost:9876/karma.js:189
        at http://localhost:9876/context.html:53

could this be a problem introduced by newer versions of browserify ?

Raynos commented 9 years ago

This no longer works with newer versions of browserify.

This module is also a bad idea (tm).

fatso83 commented 9 years ago

why is this module a bad idea? how would you otherwise test the collaborators, @Raynos?

Raynos commented 9 years ago

@fatso83 dont intercept require, ever.

Just pass the proper arguments around to do testing.

Make you interface customizable.

fatso83 commented 9 years ago

That implies a whole lot about your code structure, and is basically just advocating having a whole lot of setters (setPersistenceImpl(impl), setWebSocketImpl(impl), etc) or something similar, like optional arguments. In Java this is somewhat common for DI (dependency injection). The usual other alternative there is to use type annotations for implicit DI.

While the former works, and I have done it in some projects, you have to create a lot of dumb boilerplate code that is ever only used in tests. Using something like proxyquire (which actually works like a treat, unlike mock) makes it possible to leave the production code untouched in the few cases where you need to control the collaborators.

I do appreciate the clarity one gets by explicitly saying

var myModule = require('mymodule');
myModule.setCollab1(stub1);
myModule.setCollab2(stub2);

myModule.doStuff()
expect(stub1.called).to.be.true;

but that seems quite redundant, and not really that much clearer than what mock or proxyquire is trying to achieve. It also leaves out styles of coding where the require starts an initialization process involving other modules, leaving you with redundant init() methods littered all over.

Raynos commented 9 years ago

If you have a stateful module like that example, then your a bad person and should feel bad.

What you really want is something like

var MyModule = require('mymodule');

var myModule = MyModule({
  collab1: stub1,
  collab2: stub2
});

myModule.doStuff()
expect(stub1.called).to.be.true;

Your implementation of MyModule should look like this.

var collab1 = require('collab1')

module.exports = MyModule

function MyModule(opts) {
  this.collab1 = opts.collab1 || collab1

  if (!opts.collab2) {
    throw new Error('assume collab2 is required and cant be defaulted')
  }
  this.collab2 = opts.collab2
}
Raynos commented 9 years ago

It also leaves out styles of coding where the require starts an initialization process involving other modules, leaving you with redundant init() methods littered all over.

A stateful require is a bad require.

There should only be one stateful require, the entry point. And ideally

// main.js

module.exports = main;

function main() { ... }

if (require.main === module) {
  main();
}
fatso83 commented 9 years ago

Stateful models might not be a good thing, as they are hard to test, but they do exist and often in code one self has no control of. That's when proxyquire comes to the rescue.

It's also useful to make tests run quicker, if the requires are used anyway, as I just discovered today. I usually set a time limit of 100 ms, just to catch code that uses too much time, as most tests should complete in a few milliseconds. Today one tests failed, and I found out it was caused by requireing the request library, which in itself took 140 milliseconds. Stubbing that out fixed the tests, which weren't supposed to touch the network anyway.

I do realize it's a design smell if you need to resort to hacking require to test something, but there is a need for tools like this.

Raynos commented 9 years ago

@fatso83

If you run all your tests in the same process you can just pre-load the dependency tree before you run any tests thus avoiding the 140ms issue.

if you spend 140ms doing IO using the request library then implement DI for your module to swap out request.