Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing require result instead of variables #24

Closed
wnr opened this issue Dec 17, 2013 · 3 comments
Closed

Changing require result instead of variables #24

wnr opened this issue Dec 17, 2013 · 3 comments

Comments

@wnr
Copy link

wnr commented Dec 17, 2013

First of all: I love the way rewire is built and I really see the advantages by keeping and using an intact require function.

The only drawback i can see by doing this instead of the node-sandboxed-module approach, is that you have to keep track of variable names being used in the module being mocked. With the altering-require-function approach, one only needs to remember which module the mocked module is dependent upon.

For example, let's say we have the following module foo.js:

var helper = require('./utils.js');

If I would like to rewire the module for better testing I would do something like this:

var module = rewire('./foo.js');
module.__set__({
  helper: function() { ... } //Need to keep track & remember variable name 'helper' being used in module.
});

With node-sandboxed-module I would do something like this:

var sm = require('sandboxed-module');
var module = sm('./foo.js', {
  requires: {
    'utils.js': function() { ... } //Need to keep track & remember module dependency name.
  }
});

The second approach has two advantages in my opinion:

  • Variables are more likely to be renamed than module names.
  • If the module utils.js is required at many places in a module, assigned to different variable names, one can mock all of those variables by simply mocking the required module name.

Now, I understand that this might not be the natural way of doing this since rewire doesn't alter the require function, but I wonder if there is any way of mimicking this feature? Maybe rewire could check which variable names are assigned to the required module that the user would like to mock and then simply altering the variables for the user.

Something like this:

//In module foo.js
var helper1 = require('./utils.js');
var helper2 = require('./utils.js');
var helper3 = require('./utils.js');

//In another module wishing to mock foo.js module.
var module = rewire('./foo.js');

//Mock all variables assigned to the required module 'utils.js'
module.__set__({
  requires: {
    'utils.js': function() { ... }
  }
});

//The above call would be translated to the following:
module.__set__({
  helper1: function() { ... },
  helper2: function() { ... },
  helper3: function() { ... }
});

What do you think? :)

@jhnns
Copy link
Owner

jhnns commented Dec 17, 2013

Yes, I see the advantages. It's more likely to change a variable name than the dependency.

But I think your suggestion is impossible to implement because rewire does no static analysis of your code. Thus it doesn't know which variables are referencing which module. The only way to accomplish this is by mapping the dependency-paths on module startup like node-sandboxed-module does:

var foo = rewire("./foo.js", {
    "./util.js": utilMock
});

But if your prefer this approach I think it would be better to just use node-sandboxed-module 😉.

Personally I came to the conclusion that after all unit-tests are usually gray-box-tests. Basically you are aware of the implementation but you're trying to test it like you didn't know. If you're changing the implementation you'll likely need to adjust the unit-test as well.

I like your idea, but I don't know how to implement it.

@wnr
Copy link
Author

wnr commented Dec 18, 2013

Okay, thanks for your answer. Too bad I can't have the best of rewire and sandboxed-module. I think I'm gonna stick with rewire either way since I really like the non-eval approach :)

Interesting about the gray-box-testing. Never thought about that in that way, but I guess it makes sense.

@wnr wnr closed this as completed Dec 18, 2013
@jhnns
Copy link
Owner

jhnns commented Dec 18, 2013

Nevermind, it's always good to discuss about improvements 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants