cleishm / jsmockito

Javascript mocking framework inspired by the awesome mockito
http://jsmockito.org
Other
106 stars 19 forks source link

mock does not support constructor args #12

Closed arieljake closed 12 years ago

arieljake commented 12 years ago

If I understand what I am looking at, then these lines in jsmockito.js mean that we cannot pass args to the constructors of objects we are mocking:

MockObject.prototype = (typeof Obj == "function")? new Obj : Obj;

From my past experience using mockito, we can call mock like:

mock(Class,[arg1,arg2,...]);

does it make sense to add this to the framework?

cleishm commented 12 years ago

Mockito does not take additional arguments to pass to the constructor of a mocked object. Indeed, the constructor for the mocked object is never invoked in Mockito.

Because JavaScript is a prototypical language, JsMockito does need to invoke the object constructor (unless it's passed a fully constructed object to mock/spy). When it does call the constructor, it passes no arguments. Perhaps it would be useful to pass arguments down to this call, but compatibility with the Mockito API is not one of those reasons. It would only be necessary for constructors that require specific arguments in order to return correctly formed objects, with all the appropriate methods in place. That's somewhat unusual, and an edge case that can easily be dealt with by constructing the object oneself before passing the whole object to mock(...).

Cheers, Chris

arieljake commented 12 years ago

I discovered this while trying to mock the ServerResponse in nodejs ('http'). It expects the ServerRequest as a constructor arg, and mockito was throwing an error when I tried to mock it.

cleishm commented 12 years ago

Yes, I can see that issue. It's a side effect of allowing a constructor to be passed to mock(...) rather than only allowing prototype objects. I'm just not sure I want to take that further by allowing you to pass arguments to the constructor also - that seems like it's encouraging the wrong pattern.

Perhaps updating the documentation, to show the use of prototype objects constructed with args? E.g.

var serverResponse = mock(new ServerResponse(arg1, arg2, arg3));

Thoughts?

arieljake commented 12 years ago

Yes, just being clear about not passing constructors the way, for example, mockito works in Actionscript, would be all right. In fact, I rather prefer passing in prototype objects as you call them, which I assume means instances of the function.

On May 14, 2012, at 11:33 AM, Chris Leishman reply@reply.github.com wrote:

Yes, I can see that issue. It's a side effect of allowing a constructor to be passed to mock(...) rather than only allowing prototype objects. I'm just not sure I want to take that further by allowing you to pass arguments to the constructor also - that seems like it's encouraging the wrong pattern.

Perhaps updating the documentation, to show the use of prototype objects constructed with args? E.g.

var serverResponse = mock(new ServerResponse(arg1, arg2, arg3));

Thoughts?


Reply to this email directly or view it on GitHub: https://github.com/chrisleishman/jsmockito/issues/12#issuecomment-5698185

theone1984 commented 12 years ago

Encouraging the wrong pattern is not an argument ... don't you want that code to be tested, too? I don't think that it's a good idea to allow only 'good' code to be tested. There's so much legacy code out there that simply won't be tested when it's too much of a burden.

cleishm commented 11 years ago

Very belated response to @theone1984: This will not prevent that code from being tested. One must simply instantiate the object before calling mock(...), as in var serverResponse = mock(new ServerResponse(arg1, arg2, arg3));.