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

Cannot properly test modules that export a constructor #16

Closed
thanpolas opened this issue Jan 29, 2013 · 12 comments
Closed

Cannot properly test modules that export a constructor #16

thanpolas opened this issue Jan 29, 2013 · 12 comments

Comments

@thanpolas
Copy link

A module (A) requires a module (B) that exports a constructor (aka a class).

We want to mock "module B" and at least be able to measure:

  1. If it has been instantiated and how many times.
  2. If a method of "module B" has been invoked and how many times.

We also want to control what results the method returns or what callbacks are yielded, but let's not get ahead of ourselves at this point.

The tool we use for mocking and measuring is sinon.js.

I have tried two ways to accomplish the above task:

1. Mocking the Dependency

Mocking the dependency by using the injector.mock() method. I could not find a satisfactory solution with that.

2. Storing the Dependency

Storing the dependency using injector.store() method. Using this method there was an unexpected result. The app behaved like there was no stubbing on the provided namespace (mocks.store['moduleName']) thus providing no useful information as to if the dependency was invoked.

Demo

Please see this plnkr where the issue is illustrated.

Approach 1 is implemented in file moduleA.test.js

Approach 2 is implemented in file moduleA.test2.js

@thanpolas
Copy link
Author

@cjohansen, @mantoni if you have any feedback on this i'd be greatly appreciated

@cjohansen
Copy link

You cannot replace a "bare" function with Sinon. In order to replace something with a stub/mock implementation with Sinon, you need an object and the name of the function. However, you should be able to stub the prototype in order to monitor all the instance methods, i.e. sinon.stub(Constructor.prototype).

Beware that in Sinon, a "mock" is something that carries expectations (i.e. assertions/verification) - and should only be used in tests where you'd be OK with using an assertion. In most cases, a stub is what you want.

@joeeames
Copy link
Collaborator

this should produce a constructor that supplies a stub object that you can monitor and control.
Does that fit the bill?

var mockDependency = sinon.stub({method1: function() {}, method2: function() {});
injector.mock(['path/myDependency'], sinon.stub().returns(sinon.stub().returns(mockDependency)));

@thanpolas
Copy link
Author

@joeeames that didn't help... i tried it in the plnkr and you can view and tinker with it in the file named moduleA.test.js

@cjohansen what if this function is inside an object? e.g.

var app = {};
// the constructor
app.Constr = function(){ /* payload we don't want to be executed */ };
app.Constr.prototype.method = function(){ /* method's payload that we also don't want executed */ };

How can i measure if app.Constr has been invoked without using a sinon.spy that would result in the payload to be executed? All the while I will be able to measure how many times method() has been executed as well...

The code that needs to be tested is this:

app.operation = function() {
  var con = new Constr();
  doStuff(con.method());
};

@iammerrick did you have a look at file moduleA.test2.js and why injector.store does not behave as it should?

thank you all for your help, it's appreciated

@joeeames
Copy link
Collaborator

My solution does work.

You were asking the stub for the wrong thing. you were asking the stub object for it's call count, but a stub object is just an object that has a bunch of stub methods. those methods are what you would want to ask for their call count. So in this case you want to ask the stub object's method named "method" for it's call count. i.e.

mockDependency.method.callCount

here is the fix:
http://plnkr.co/edit/K9iFQPufRDaih75eDElo

that has the one change in ModuleA.test.js that is required for it to work.

@thanpolas
Copy link
Author

@joeeames yes i see now, awesome! 👍

How can i see how many times mockDependency has been called?

@thanpolas
Copy link
Author

@joeeames I got it now, i have to store the reference returned by the second statement you mentioned:

var stubMethod = sinon.stub({method1: function() {}, method2: function() {});
var stubConstructor = sinon.stub().returns(stubMethod);

injector.mock(['path/myDependency'], function() { return stubConstructor; });

/* ... */

stubConstructor.callCount === 1;

That solves my problem! Thanks! :)

However the issue with the weird injector.store() behavior is still open for @iammerrick to check out...

@thanpolas
Copy link
Author

I updated the plnkr's "moduleA.test.js" file with @joeeames's solution, it works as expected.

@joeeames
Copy link
Collaborator

joeeames commented Feb 1, 2013

Ok, I looked into issue 2, the injector.store().

the problem, is that that functionality of squire only works when your dependency is returning an object, and you want to stub out the methods on that object. when it returns a function (or a class constructor, which is just a function) then when you call "var sinonModuleB = sinon.stub(mocks.store, 'moduleB');" you are actually giving the mocks object returned by squire an entirely different object. AMD still has the old object and you are no longer dealing with that instance. you have simply broken the "mocks" object's pointer at that object and given it a different object instead.

See the attached diagram for a more visual explanation.

so in short, this functionality isn't available with squire.

I'm going to close this issue.
Screen Shot 2013-02-01 at 11 14 53 AM 1

@joeeames joeeames closed this as completed Feb 1, 2013
@thanpolas
Copy link
Author

aha, i see.

Thank you for the clarification @joeeames, i guess it's only through squire.mock that we can properly stub dependencies that export a function (constructor).

Thanks!

@iammerrick
Copy link
Owner

@thanpolas Just added https://github.com/iammerrick/Squire.js#squirehelpersreturnsany-what to help with the boilerplate associate with testing constructor functions.

@thanpolas
Copy link
Author

awesome @iammerrick! Will employ it next chance i get.

In the meantime i've started a campaign to decriminalize namespaces =)

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

4 participants