-
-
Notifications
You must be signed in to change notification settings - Fork 770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sandbox.stub fails to stub methods on the prototype #1512
Comments
Thank you for your bug report, but I have no idea what "an es6 function" is. Do you mean an "arrow function" - a function whose context is the environment in which it was defined, or are you thinking about the proposed standard for class methods in esnext that are pre-bound to the instance? Or something else entirely? Not getting wiser by the prose, I am afraid. A reproducible snippet of code speaks more than three paragraphs of text :-) |
My apologies 😊 By ES6 function, I mean defining a member function with the ES6 class syntax, as we do here: One of the tests we're seeing with this issue is here: As the first link shows, the function does exist, but it's not enumerated in the But it does seem that getting the prototype of the object and checking the members of that does find the member function, as well as any defined int he old ES5 way (directly manipulating the prototype). |
Minimal reproduction of the issue described: https://gist.github.com/AccaliaDeElementia/c68ddbc0b49d12e1eec9079a28370c68 |
Hmm ... thanks for the updates and the reproducible snippet. Having the snippet means we can automate the process of finding the commit that introduced the regression (using It is true that the property is not the object's own property, rather it's further up the prototype, but since this was working before we should fix it. I suspect it has something to do with the getters and setters functionality, which would require this check. The meantime fix is simple. Just create a stub on the object like this |
When you are working with typescript that fix doesn't work because when I tried to put object.myMethod = sinon.stub I get a typescript error |
@joseSantacruz That's a limitation of Typescript. When I worked with TS I still wrote all the tests in js to make life simpler :-) You can still "legally" stub and spy on the prototype's method in the meantime. Or just use the last stable 2.x release 😬 If you have time you can help us track down the regression using the method mentioned above. |
@mroderick I used @AccaliaDeElementia's test case and found the commit that introduced the regression: The test caseconst sinon = require('sinon')
class TestObject {
someFunction() {
console.error('sinon should stub me!');
}
}
const sandbox = sinon.sandbox.create();
const instance = new TestObject();
const stub = sandbox.stub(instance, 'someFunction'); // This fails on Sinon 3.0, it worked on Sinon 2.x
It seems something happened in that commit that did more than just remove deprecated functionality. It also means this is affecting 2.4.1, not just 3.0. |
Ah, just found out that this is purely affecting the sandbox stubbing functionality. The original report talks about |
I have a fix for this coming up. The proposed fix |
Released as Sinon 3.1 on NPM. |
please tell me this was fixed for sinon 2.4.* 😭 |
@chris-fran are you unable to upgrade? |
What did you expect to happen?
I expected that calling
sandbox.stub(object, 'property')
, whereproperty
is a function on the prototype, would result in the function being successfully stubbed with a no-op stub.What actually happens
We receive the exception
TypeError: Cannot stub non-existent own property <property>
How to reproduce
(Test case added August 7 by @fatso83)
Diagnosis
It appears the issue is in the check on like 89 of collections.js, specifically the call
Object.prototype.hasOwnProperty.call(object, property)
. It appears thathasOwnProperty()
doesn't enumerate ES6 functions and properties.Proposed fix
Instead of calling the above, use the test
Object.getOwnPropertyNames(Object.getPrototypeOf(object)).indexOf(property) > -1
. Our testing indicates that this method does enumerate ES6 functions and properties in addition to ES5 functions and properties.The text was updated successfully, but these errors were encountered: