diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 385fa431d..d5a925a94 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -218,6 +218,7 @@ function Sandbox() { delete object[property]; } } + restorer.object = object; restorer.property = property; return restorer; @@ -355,8 +356,7 @@ function Sandbox() { }; function commonPostInitSetup(args, spy) { - const object = args[0]; - const property = args[1]; + const [object, property, types] = args; const isSpyingOnEntireObject = typeof property === "undefined" && typeof object === "object"; @@ -369,6 +369,11 @@ function Sandbox() { }); usePromiseLibrary(promiseLib, ownMethods); + } else if (Array.isArray(types)) { + for (const accessorType of types) { + addToCollection(spy[accessorType]); + usePromiseLibrary(promiseLib, spy[accessorType]); + } } else { addToCollection(spy); usePromiseLibrary(promiseLib, spy); diff --git a/lib/sinon/util/core/get-property-descriptor.js b/lib/sinon/util/core/get-property-descriptor.js index edd03dbe3..125619724 100644 --- a/lib/sinon/util/core/get-property-descriptor.js +++ b/lib/sinon/util/core/get-property-descriptor.js @@ -1,6 +1,28 @@ "use strict"; -module.exports = function getPropertyDescriptor(object, property) { +/* eslint-disable jsdoc/valid-types */ +/* + * The following type def is strictly an illegal JSDoc, but the expression forms a + * legal Typescript union type and is understood by Visual Studio and the IntelliJ + * family of editors. The "TS" flavor of JSDoc is becoming the de-facto standard these + * days for that reason (and the fact that JSDoc is essentially unmaintained) + */ + +/** + * @typedef {{isOwn: boolean} & PropertyDescriptor} SinonPropertyDescriptor + * a slightly enriched property descriptor + * @property {boolean} isOwn true if the descriptor is owned by this object, false if it comes from the prototype + */ +/* eslint-enable jsdoc/valid-types */ + +/** + * Returns a slightly modified property descriptor that one can tell is from the object or the prototype + * + * @param {*} object + * @param {string} property + * @returns {SinonPropertyDescriptor} + */ +function getPropertyDescriptor(object, property) { let proto = object; let descriptor; const isOwn = Boolean( @@ -19,4 +41,6 @@ module.exports = function getPropertyDescriptor(object, property) { } return descriptor; -}; +} + +module.exports = getPropertyDescriptor; diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index 6a1b88b05..a8b30328c 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -899,4 +899,19 @@ describe("issues", function () { this.sandbox.restore(); }); }); + + it("#2534 - spies on accessors are not being cleaned up", function () { + const object = { + get prop() { + return "bar"; + }, + }; + const spy = sinon.spy(object, "prop", ["get"]); + /* eslint-disable no-unused-expressions */ + object.prop; + assert.equals(spy.get.callCount, 1); + sinon.restore(); + object.prop; + assert.equals(spy.get.callCount, 1); // should remain unchanged + }); }); diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 9c90cfc86..9ea67307f 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -2205,7 +2205,7 @@ describe("Sandbox", function () { assert.equals(object.prop, "blabla"); }); - it("allows restoring setters", function () { + it("allows putting setters on fields and subsequently restoring them", function () { const object = { prop: "bar", }; @@ -2221,6 +2221,37 @@ describe("Sandbox", function () { assert.equals(object.prop, "bla"); }); + + it("allows replacing setters on fields and subsequently restoring them", function () { + const object = { + get prop() { + return "bar"; + }, + }; + + const sandbox = new Sandbox(); + const getter = sandbox.spy(() => "foobar"); + sandbox.stub(object, "prop").get(getter); + assert.equals(object.prop, "foobar"); + assert.equals(getter.callCount, 1); + + sandbox.restore(); + assert.equals(object.prop, "bar"); + }); + + it("allows spying on accessors and subsequently restoring them", function () { + const object = { + get prop() { + return "bar"; + }, + }; + const sandbox = new Sandbox(); + const spy = sandbox.spy(object, "prop", ["get"]); + sandbox.restore(); + const descriptor = Object.getOwnPropertyDescriptor(object, "prop"); + const getter = descriptor.get; + refute.equals(getter, spy.get); + }); }); describe(".assert", function () {