spy.wrappedMethod is not ok when spying getter/setter

[NoamDev]

Describe the bug
when spying getter/setter, spy.get.wrappedMethod and spy.set.wrappedMethod are both undefined but spy.wrappedMethod is the original getter/setter whoever was written last (looks like an override).

To Reproduce

var object = {
  get test() {
    return 1;
  },
  set test(value) {
  }
};

var stub = sinon.spy(object, "test",['get','set']);

console.log('1',stub.get.wrappedMethod)
console.log('2', stub.set.wrappedMethod)
console.log('3', stub.wrappedMethod)
stub.restore()
var stub = sinon.spy(object, "test",['set','get']);
console.log('4', stub.wrappedMethod)

results can be seen here https://runkit.com/embed/b9qjkhx4ufk7

Expected behavior
the first should be the original getter
the second should be the original setter
the third should be the undefined
the fourth should be the undefined

Actual behavior
the first is undefined
the second is undefined
the third is the original setter
the fourth is the original getter

[fatso83]

Before anyone tries to fix this, I think we should think a bit about how this feature is supposed to work so it doesn't become a architectural patchwork

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[NoamDev]

Ahhh, don't let it become stale....
It was documented for stubs..

[tomalec]

It was documented for stubs but is also not supported for stubs.

@fatso83 Could you elaborate on what needs to be specified from the architectural perspective?
Was there something missing in https://github.com/sinonjs/sinon/pull/2251/files?
Forgive me if these are dumb questions, but I'm new to the Sinon codebase.

From a user/DevX perspective to me, setters and getters are functions like any others, so I'd expect the same spy/stub capabilities I have for other functions.

The property descriptor is something special, that may require a different approach, but that's out of the scope of this issue, isn't it?

My use-case is more or less:

let obj = {
	funName: function fun() {
		return 'Works';
	}
}
Object.defineProperty( obj, 'prop', {
	get: function propGetter() {
		return 'boo';
	},
	configurable: true
})
let funStub = sinon.stub(obj, 'funName').callsFake( function fakedFun(){
	return funStub.wrappedMethod.apply(this, arguments) + ' like charm!';
})
console.log( obj.funName() ); // Works like charm!

let getterStub = sinon.stub(obj, 'prop').get( function fakedGetter(){
	return getterStub.wrappedMethod.apply(this, arguments) + ' like charm!';
})
console.log( obj.prop ); // Cannot read property 'apply' of undefined

I don't see a reason why stubbed getter cannot work the same as any other stubbed function.

[fatso83]

@tomalec Sorry for not responding, but I have little time to contribute these days. I barely remember anything about the implementation details; you need to invest some time when tinkering with this to understand it, but I do remember that we have had multiple iterations on getters and setters, and that we have not been that great at documenting how they work and which parts of the API is official and not.

I would just try and scratch your itch, if I were you, and make a PR or POC or something that could be discussed. TBH, I never use anything but the sinon.fake API anymore, as it keeps my tests simpler. And I don't think I have used getters and setters since 2015, as I found they made the code more magic than cool. So something like this should be led by someone who actually uses such a feature, not me :)

[fatso83]

This discussion is related and useful, btw: #1741 (comment)

[fatso83]

Ah, just noticed this has been filled by #2251 (which needs to be reviewed by someone).

[fatso83]

This should be fixed by @rgroothuijsen patches in #2378