Feature: getters & setters

[simonzack]

This PR addresses #88. I didn't touch mock as I've read about the plans to remove it in 2.0.

I've tried to make as little modification to the existing codebase as possible, and integrated getters & setters by providing an additional type parameter, which can take the value get or set. Using defineProperty and friends also solves #690.

Example usage:

var obj = {};
Object.defineProperty(obj, 'property', {
  get: function(){
    return 1;
  },
  configurable: true
});
var stub = sinon.stub(obj, 'get', 'property', function(){return 2;});
console.log(obj.property);
stub.restore();

@cjohansen I understand that you're not really a fan of getters & setters, but a lot of people do have a need for this (see above issue). Since sinon is the best (and only) mocking framework around, I think that getter & setter support will really benefit the community.

[cjohansen]

My issue with this is that if you need to stub out properties, you probably shouldn't be using getters, but functions. Getters are nice for accessing private data or making quick computations over other data, but in my mind, anything complicated enough warranting stubbing out in tests seems just like bad design for me. It has always been a design goal for Sinon not to enable or make it too easy to do stupid things.

Can you come up with some convincing use cases for this? If we do go for this, and I think @mantoni and @mroderick should pitch in as well, I'm not ecstatic about the suggested API. I think we should either have a dedicated function, like sinon.stubProperty or maybe something like:

sinon.stub(object, 'property', propertyDescriptor);

That last one will be a little more verbose, which is a good thing. Maybe it'll make people think twice.

As a last note, I personally feel that a better solution is to have a section on stubbing properties in the docs, laying down the case that you shouldn't do it, would be preferable. But it's not up to me alone.

[mantoni]

Like @cjohansen, I wouldn't want to add an API for this, for the said reasons. However, I understand the need to stub a property that was defined with a getter. I'd use the existing sandbox functionality, but this does not work at the moment:

var sinon = require('sinon');

var o = {};
Object.defineProperty(o, 'p', { get : function () { return 1; }, configurable : true });

var s = sinon.sandbox.create();
s.stub(o, 'p', 2); // TypeError: Cannot set property p of #<Object> which has only a getter

[simonzack]

My own use case of this is to stub a configuration class, which abstracts over a key value store. Getters and setters are used to interact with the store. I then provide the getter with a value, test whether the setter is called with the appropriate value, called only once, etc. I could use functions instead, but I personally find the getter/setter approach to be simpler.

I think this is more of a design preference, but if it is not convincing enough, I'm sure many others would also like the chime in here.

I agree with @mantoni that adding new APIs is not ideal, but I cannot see how both getters and setters can be used with the existing api. I'll update my PR with @cjohansen last api as it does look a lot better than mine.

[simonzack]

@cjohansen I've switched over now to your last interface.

[cjohansen]

Build is failing

[simonzack]

@cjohansen Could you run the build again? Only 0.10 is failing and it says "this potentially indicates a stalled build", could be a travis issue.

[mroderick]

Re-started the build, it passed

[cjohansen]

Thanks for your work on this. Let's hope people won't use it too much ;)

[hswolff]

Just a heads up, but this change broke sinon.js for IE<9 as it uses ES5 methods. Trying to fix it locally, it's a current WIP.

[sungwoncho]

Is this documented yet?

[vitalets]

+1 for documentation

[vitalets]

Finally working example to stub getter:

var o = {
    get foo() { return 'foo' }
};

sinon.stub(o, 'foo', { get: function () { return 'foo stubbed' }});
console.log(o.foo); // foo stubbed

[novemberborn]

In the example above, sinon.stub(o, 'foo', { get: function () { return 'foo stubbed' }}) returns a spy. As far as I can tell it's not possible to change the behavior of the getter without replacing it entirely. Is this intentional?

Similarly when using sinon.stub({ get foo () {} }) it won't stub the getter, it only stubs functions on the object.

[fatso83]

@novemberborn : I do not know. Maybe some of the others might.

[fatso83]

@simonzack This seems to have introduced a regression affecting Chrome. See #1046. Could you have a look?