Make stub work for prototype methods

[felixge]

This is incredibly useful if you work with named constructors and
prototype methods.

All my classes look like this:

function MyClass() {}

MyClass.prototype.myFunc = function() {};

Without this patch sinon.stub(new MyClass) does not stub my prototype methods.

Let me know what you think!

[rmehner]

+1 to that. We had this discussion over twitter:

https://twitter.com/#!/cjno/status/126676122242007040
https://twitter.com/#!/cjno/status/126676251661443072
https://twitter.com/#!/cjno/status/126676369903067136

While the argument "I don't want Sinon to help you successfully test an object that uses a collaborator in a way that fails in production" is totally valid, I still think there should be a way to stub objects with prototype methods too.

If not within the stub method then maybe in something like "stubWithPrototype" (bit verbose though).

[cjohansen]

If we're going to allow stubbing inherited properties, is there any point in restricting it to the first object on the prototype chain? I.e. do we need the isOwnProperty || isProtoProperty check? Wouldn't an unfiltered for-in be just as good?

[felixge]

If we're going to allow stubbing inherited properties, is there any point in restricting it to the first object on the prototype chain? I.e. do we need the isOwnProperty || isProtoProperty check? Wouldn't an unfiltered for-in be just as good?

That would work for me. Generally if I do sinon.stub(obj) I intend to stub all methods on that object, regardless of where they came from. If I want fine grained control, I will generally stub individual methods instead.

Let me know and I will modify the this patch.

[cjohansen]

I think I agree. I'm not entirely sure why the hasOwnProperty check is in there. What I don't want Sinon to do is to fake non-existing properties (i.e. sinon.stub(obj, "nonExistentProperty")). I don't actually mind faking inherited methods.

Pull out the filter, and I'll pull it!

[felixge]

Btw. I just figured out that another rather doable way is:

var stub = sinon.stub(Object.create(MyClass.prototype));

But generally I would probably still prefer what's discussed above as it feels more natural to do the stubbing that way.

[cjohansen]

That's definitely less easy on the eyes :) But I'll pull your change if you pick out the filter, so it just stubs whatever method the object has, regardless of prototype chain origin.

[felixge]

That's definitely less easy on the eyes :)

Turns out it wasn't actually working with an unpatched version of sinon either (npm was doing unexpected things, see https://github.com/isaacs/npm/issues/1727).

Anyway, I submitted the discussed simplification above, all tests passing!

[felixge]

Sorry for all the little commits, I can squash this into one if you'd like me to. But should be good now.

[cjohansen]

No, I like small commits :) Thanks again!

[felixge]

Sweet, thanks a lot!

[jareware]

This makes perfect sense! So much sense, in fact, that I'm confused spies don't do the same; shouldn't they share most of their API..?

I'm on latest stable.

Cheers!

[cjohansen]

@jareware I guess that would make sense. I'll gladly merge it if you want to fix it.