Skip to content
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

Handling non-configurable object descriptors on the prototype #2508

Merged
merged 6 commits into from Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/sinon/stub.js
Expand Up @@ -134,7 +134,7 @@ function assertValidPropertyDescriptor(descriptor, property) {
if (!descriptor || !property) {
return;
}
if (!descriptor.configurable && !descriptor.writable) {
if (descriptor.isOwn && !descriptor.configurable && !descriptor.writable) {
throw new TypeError(
`Descriptor for property ${property} is non-configurable and non-writable`
);
Expand Down
1 change: 1 addition & 0 deletions lib/sinon/util/core/wrap-method.js
Expand Up @@ -137,6 +137,7 @@ module.exports = function wrapMethod(object, property, method) {
for (i = 0; i < types.length; i++) {
mirrorProperties(methodDesc[types[i]], wrappedMethodDesc[types[i]]);
}
methodDesc.configurable = true;
Object.defineProperty(object, property, methodDesc);

// catch failing assignment
Expand Down
51 changes: 51 additions & 0 deletions test/issues/issues-test.js
Expand Up @@ -822,4 +822,55 @@ describe("issues", function () {
assert.isTrue(fooStubInstance.wasCalled);
});
});

describe("#2491 - unable to restore spies on an instance where the prototype has an unconfigurable property descriptor", function () {
function createInstanceFromClassWithReadOnlyPropertyDescriptor() {
class BaseClass {}
Object.defineProperty(BaseClass.prototype, "aMethod", {
value: function () {
return 42;
},
});

// anchor
const instance = new BaseClass();
assert.equals(instance.aMethod(), 42);

return instance;
}

it("should ensure copied object descriptors are always configurable for spies", function () {
const instance =
createInstanceFromClassWithReadOnlyPropertyDescriptor();
this.sandbox.spy(instance, "aMethod");

refute.exception(() => {
this.sandbox.restore(); // #2491: this throws TypeError: Cannot assign to read only property 'myMethod' of object '#<BaseClass>'
});
});

it("should not throw if the unconfigurable object descriptor to be used for a Stub is on the prototype", function () {
const instance =
createInstanceFromClassWithReadOnlyPropertyDescriptor();

// per #2491 this throws 'TypeError: Descriptor for property aMethod is non-configurable and non-writable'
// that makes sense for descriptors taken from the object, but not its prototype, as we are free to change
// the latter when setting it
refute.exception(() => {
this.sandbox.stub(instance, "aMethod").returns("a stub");
});
});

it("should not throw if the unconfigurable object descriptor to be used for a Mock is on the prototype", function () {
const instance =
createInstanceFromClassWithReadOnlyPropertyDescriptor();

const mock = this.sandbox.mock(instance);

// per #2491 this throws 'TypeError: Attempted to wrap undefined property myMethod as function
refute.exception(() => {
mock.expects("aMethod").once();
});
});
});
});