Sinon: Wrap ES5 getters and setters

[rvagg]

var o = {
  get foobar() { return 'foobar' },
  set foobar(s) { throw 'bzzt!' },
  foo: function() { return 'foo' },
  bar: 'bar'
}; // -> { foobar: [Getter/Setter], foo: [Function], bar: 'bar' }

sinon.mock(o).expects('foobar').returns('woohoo'); // doesn't because sinon.wrapMethod() does an object[property] which accesses the getter which returns a string

To support getters and setters you need to do a Object.getOwnPropertyDescriptor(obj, prop) for the object and then a recursive Object.getPrototypeOf(obj) if it doesn't exist, the descriptor has a 'get' and 'set' property where they are defined.

But, you may want to mock out just the getter or just the setter or both, so how do you specify that? Some options:

mock(o).expects('foobar'); // ambiguous, the setter? getter? or both?
mock(o).expects('get foobar'); // ambiguous, there may be a legitimate property named "get foobar"
mock(o).expectsGet('foobar'); // clear but more code in Sinon and more to document ?
mock(o).expectsSet('foobar'); // ditto

And also, do you have a convention in place already for dealing with features that may not be present in the current environment? If someone tries to do this in IE6 what should happen?

[cjohansen]

Looking at your examples, I'm not entirely sure that mocking getters is good testing practice - I would assume that simply setting the property to some value for the duration of the test would go a long way in most cases(?)

Anyway, what if the stub is an object instead of a function, it will be treated as a property descriptor? Something like:

stub(o, "foobar", { get: function () { return 42; } });

I'm not sure how to resolve your expectations though. But like I said - but is it worthwhile putting mock expectations on property lookups? Seems to me that if you need that, maybe they should be "actual" methods instead?

[rvagg]

The example was simplistic. The real code I was trying to do this on was called get identifier which appended a few things together, some of which came from places that I didn't want to bother with in the test, I just wanted to confirm that x.identifier was accessed and then have control over what was returned. For now I'm going with a getIdentifier() which is fine but given that we may be seeing more of getters and setters into the future for computed properties this may be something Sinon could/should handle?

[cjohansen]

Still kinda undecided on this. Will let it rest for a while and decide later. If you have more input/suggestions, feel free to jot them down here.

[dmose]

One use case that's not uncommon is where a getter is provided but a setter is not, which makes the property read-only, which may well be a good idea for production code, but get in the way of testing. Having Sinon support getters and setters would speak to that use case.

[cjohansen]

For the time being, I don't see the value in supporting getters and setters in Sinon.

[felixhammerl]

obviously, it's been a while since this issue has been discussed, but this feature would be very helpful...

[giggio]

+1
I also miss it. It would be specially useful when used with components that work as abstractions with getters over some underlying resource, like files, or database objects. It would be nice to be able to stub those getters.

[qubyte]

I'd also like to see this reopened. It'd be useful specifically for stubbing fields of a wrapped location object.

[vitalets]

+1 for that feature!

[nunofgs]

:+1: I also just felt the need for this feature.

[simonzack]

:+1: This needs to be re-opened. Especially with ES6 catching on, this is more important than ever. I like the expectsGet variant.

[markcellus]

+1 I wholeheartedly agree with @simonzack

[machineghost]

+1

[mroderick]

I'd also like to see this reopened. It'd be useful specifically for stubbing fields of a wrapped location object.

Does https://github.com/mroderick/wrapple plug that gap for you?

[qubyte]

Nowadays I use wrapple quite extensively, and I'd forgotten about commenting on this issue. That probably means that wrapple has plugged the gap as you say.

[derwaldgeist]

+1 just stumbled upon sinon not supporting getter mocking

[lucasfcosta]

Hi @derwaldgeist, are you using Sinon's latest version?

The latest one supports stubbing getters with both sandboxes and sinon.stub.

All you gotta do is use the get function, for example:

var myObj = {
    prop: "foo"
};

createStub(myObj, "prop").get(function getterFn() {
    return "bar";
});

myObj.prop // "bar"

Please let me know if this suits your use case or if you have any further doubts.

[derwaldgeist]

Thanks for coming back that quick, highly appreciated.
However, I did not find how you can restore a getter that has been defined that way? I even looked at the sourcecode of sinon and could not see a mechanism that stores the original getter when calling get().

[lucasfcosta]

Hi @derwaldgeist,

It's indeed possible to restore getters, even if they were undefined before being stubbed. All you gotta do is call the restore method in the created stub, for example:

var myObj = {
    prop: "foo"
};

var stub = createStub(myObj, "prop");

stub.get(function getterFn() {
    return "baz";
});

myObj.prop // "baz"

stub.restore();

myObj.prop // "foo"

We add this restore method that either in the stub method itself (when stubbing a non-function property) or in the wrapMethod utility.

[derwaldgeist]

Wow, that was fast. I tried this:

 const stub = sinon.stub(myObj, 'prop');
 stub.get(() => ({}));
  ...
 stub.restore();

but when executing restore() it says: Cannot redefine property: prop

Where does your createStub method come from?

[lucasfcosta]

@derwaldgeist I got that from a test, sorry, forgot to edit it. But anyway, it is the same as sinon.stub.

Can you tell me what version of sinon are you using so that I can check it?

I think that maybe you are not using the latest version yet in which we fixed a bug where you could not restore properties due to them being defined without configurable: true and therefore we were not able to change their descriptors.

[JakobJingleheimer]

I'm seeing the same as @derwaldgeist. I _think_ I'm using v4, but some other deps were using v1.7, so I'm not sure which is _actually_ getting used. I didn't see a sinon.version where i could check at run time.

[fatso83]

@jshado1 If you want to know which major version you are on, you can refer to the changelog or migration guide and test for the breaking changes. For version 4, you can assert that sinon.stub({}, 'nonExistingProperty') throws