From 70bfa38b3a3898e92e735cfebfc8209761ecf9b1 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Fri, 24 Mar 2023 18:25:15 +0100 Subject: [PATCH 1/4] fix: make it possible to call through to underlying stub in stub instances refs #2477 refs #2501 --- lib/sinon/stub.js | 8 +++----- lib/sinon/util/core/wrap-method.js | 7 +++++++ test/issues/issues-test.js | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 8b9e32af7..1509642d3 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -131,12 +131,10 @@ stub.createStubInstance = function (constructor, overrides) { throw new TypeError("The constructor should be a function."); } - // eslint-disable-next-line no-empty-function - const noop = () => {}; - const defaultNoOpInstance = Object.create(constructor.prototype); - walkObject((obj, prop) => (obj[prop] = noop), defaultNoOpInstance); + const stubInstance = Object.create(constructor.prototype); + stubInstance[Symbol.for("SinonType")] = "StubInstance"; - const stubbedObject = stub(defaultNoOpInstance); + const stubbedObject = stub(stubInstance); forEach(Object.keys(overrides || {}), function (propertyName) { if (propertyName in stubbedObject) { diff --git a/lib/sinon/util/core/wrap-method.js b/lib/sinon/util/core/wrap-method.js index fc2cd4815..99ddd47a2 100644 --- a/lib/sinon/util/core/wrap-method.js +++ b/lib/sinon/util/core/wrap-method.js @@ -1,5 +1,7 @@ "use strict"; +// eslint-disable-next-line no-empty-function +const noop = () => {}; var getPropertyDescriptor = require("./get-property-descriptor"); var extend = require("./extend"); var hasOwnProperty = @@ -230,6 +232,11 @@ module.exports = function wrapMethod(object, property, method) { } } } + if (object[Symbol.for("SinonType")] === "StubInstance") { + // this is simply to avoid errors after restoring if something should + // traverse the object in a cleanup phase, ref #2477 + object[property] = noop; + } } return method; diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index aa80f8f57..ea3a1993f 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -805,4 +805,21 @@ describe("issues", function () { assert.isUndefined(restoredPropertyDescriptor); }); }); + + describe("#2501 - createStubInstance stubs are not able to call through to the underlying function on the prototype", function () { + it("should be able call through to the underlying function on the prototype", function () { + class Foo { + testMethod() { + this.wasCalled = true; + return 42; + } + } + + const fooStubInstance = this.sandbox.createStubInstance(Foo); + fooStubInstance.testMethod.callThrough(); + // const fooStubInstance = new Foo() + fooStubInstance.testMethod(); + // assert.isTrue(fooStubInstance.wasCalled); + }); + }); }); From 34aab853504655e14b7d3098f2ee04514ec1f7df Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 26 Mar 2023 12:03:42 +0200 Subject: [PATCH 2/4] internal: Extract underlying createStubInstance --- lib/sinon/create-stub-instance.js | 35 +++++++++++++++++++++++++++++++ lib/sinon/sandbox.js | 3 ++- lib/sinon/stub.js | 27 ------------------------ test/stub-test.js | 2 +- 4 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 lib/sinon/create-stub-instance.js diff --git a/lib/sinon/create-stub-instance.js b/lib/sinon/create-stub-instance.js new file mode 100644 index 000000000..23baf4fb0 --- /dev/null +++ b/lib/sinon/create-stub-instance.js @@ -0,0 +1,35 @@ +"use strict"; + +const stub = require("./stub"); +const forEach = require("@sinonjs/commons").prototypes.array.forEach; + +function isStub(value) { + return value && value.throws && value.returns; +} + +module.exports = function createStubInstance(constructor, overrides) { + if (typeof constructor !== "function") { + throw new TypeError("The constructor should be a function."); + } + + const stubInstance = Object.create(constructor.prototype); + stubInstance[Symbol.for("SinonType")] = "StubInstance"; + + const stubbedObject = stub(stubInstance); + + forEach(Object.keys(overrides || {}), function (propertyName) { + if (propertyName in stubbedObject) { + var value = overrides[propertyName]; + if (isStub(value)) { + stubbedObject[propertyName] = value; + } else { + stubbedObject[propertyName].returns(value); + } + } else { + throw new Error( + `Cannot stub ${propertyName}. Property does not exist!` + ); + } + }); + return stubbedObject; +}; diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 66fbe1c6f..772d67499 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -11,6 +11,7 @@ var sinonClock = require("./util/fake-timers"); var sinonMock = require("./mock"); var sinonSpy = require("./spy"); var sinonStub = require("./stub"); +var sinonCreateStubInstance = require("./create-stub-instance"); var sinonFake = require("./fake"); var valueToString = require("@sinonjs/commons").valueToString; var fakeServer = require("nise").fakeServer; @@ -71,7 +72,7 @@ function Sandbox() { }; sandbox.createStubInstance = function createStubInstance() { - var stubbed = sinonStub.createStubInstance.apply(null, arguments); + var stubbed = sinonCreateStubInstance.apply(null, arguments); var ownMethods = collectOwnMethods(stubbed); diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 1509642d3..0edc88d5a 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -126,33 +126,6 @@ function stub(object, property) { return isStubbingNonFuncProperty ? s : wrapMethod(object, property, s); } -stub.createStubInstance = function (constructor, overrides) { - if (typeof constructor !== "function") { - throw new TypeError("The constructor should be a function."); - } - - const stubInstance = Object.create(constructor.prototype); - stubInstance[Symbol.for("SinonType")] = "StubInstance"; - - const stubbedObject = stub(stubInstance); - - forEach(Object.keys(overrides || {}), function (propertyName) { - if (propertyName in stubbedObject) { - var value = overrides[propertyName]; - if (value && value.createStubInstance) { - stubbedObject[propertyName] = value; - } else { - stubbedObject[propertyName].returns(value); - } - } else { - throw new Error( - `Cannot stub ${propertyName}. Property does not exist!` - ); - } - }); - return stubbedObject; -}; - function assertValidPropertyDescriptor(descriptor, property) { if (!descriptor || !property) { return; diff --git a/test/stub-test.js b/test/stub-test.js index 2537a2f61..6524a3822 100644 --- a/test/stub-test.js +++ b/test/stub-test.js @@ -2,7 +2,7 @@ var referee = require("@sinonjs/referee"); var createStub = require("../lib/sinon/stub"); -var createStubInstance = require("../lib/sinon/stub").createStubInstance; +var createStubInstance = require("../lib/sinon/create-stub-instance") var createSpy = require("../lib/sinon/spy"); var createProxy = require("../lib/sinon/proxy"); var match = require("@sinonjs/samsam").createMatcher; From 7d60f2d43cacbc7bc18cd2b668fffd0c3f534939 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 26 Mar 2023 12:07:54 +0200 Subject: [PATCH 3/4] internal: extract tests into own module --- test/create-stub-instance-test.js | 158 ++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 test/create-stub-instance-test.js diff --git a/test/create-stub-instance-test.js b/test/create-stub-instance-test.js new file mode 100644 index 000000000..c7db51ce9 --- /dev/null +++ b/test/create-stub-instance-test.js @@ -0,0 +1,158 @@ +"use strict"; + +var referee = require("@sinonjs/referee"); +var createStub = require("../lib/sinon/stub"); +var createStubInstance = require("../lib/sinon/create-stub-instance") +var assert = referee.assert; +var refute = referee.refute; + +describe("createStubInstance", function () { + it("stubs existing methods", function () { + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + + var stub = createStubInstance(Class); + stub.method.returns(3); + assert.equals(3, stub.method()); + }); + + it("throws with no methods to stub", function () { + var Class = function () { + return; + }; + + assert.exception( + function () { + createStubInstance(Class); + }, + { + message: + "Found no methods on object to which we could apply mutations", + } + ); + }); + + it("doesn't call the constructor", function () { + var Class = function (a, b) { + var c = a + b; + throw c; + }; + Class.prototype.method = function () { + return; + }; + + var stub = createStubInstance(Class); + refute.exception(function () { + stub.method(3); + }); + }); + + it("retains non function values", function () { + var TYPE = "some-value"; + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + Class.prototype.type = TYPE; + + var stub = createStubInstance(Class); + assert.equals(TYPE, stub.type); + }); + + it("has no side effects on the prototype", function () { + var proto = { + method: function () { + throw new Error("error"); + }, + }; + var Class = function () { + return; + }; + Class.prototype = proto; + + var stub = createStubInstance(Class); + refute.exception(stub.method); + assert.exception(proto.method); + }); + + it("throws exception for non function params", function () { + var types = [{}, 3, "hi!"]; + + for (var i = 0; i < types.length; i++) { + // yes, it's silly to create functions in a loop, it's also a test + // eslint-disable-next-line no-loop-func + assert.exception(function () { + createStubInstance(types[i]); + }); + } + }); + + it("allows providing optional overrides", function () { + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + + var stub = createStubInstance(Class, { + method: createStub().returns(3), + }); + + assert.equals(3, stub.method()); + }); + + it("allows providing optional returned values", function () { + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + + var stub = createStubInstance(Class, { + method: 3, + }); + + assert.equals(3, stub.method()); + }); + + it("allows providing null as a return value", function () { + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + + var stub = createStubInstance(Class, { + method: null, + }); + + assert.equals(null, stub.method()); + }); + + it("throws an exception when trying to override non-existing property", function () { + var Class = function () { + return; + }; + Class.prototype.method = function () { + return; + }; + + assert.exception( + function () { + createStubInstance(Class, { + foo: createStub().returns(3), + }); + }, + { message: "Cannot stub foo. Property does not exist!" } + ); + }); +}) From eb02fb640fee1d00eed968eb033ca1575725f867 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 26 Mar 2023 12:39:37 +0200 Subject: [PATCH 4/4] internal: extract sinon type checking into own module --- lib/sinon/create-stub-instance.js | 5 +- lib/sinon/stub.js | 3 + lib/sinon/util/core/sinon-type.js | 22 +++++ lib/sinon/util/core/wrap-method.js | 3 +- test/create-stub-instance-test.js | 4 +- test/stub-test.js | 152 ----------------------------- 6 files changed, 32 insertions(+), 157 deletions(-) create mode 100644 lib/sinon/util/core/sinon-type.js diff --git a/lib/sinon/create-stub-instance.js b/lib/sinon/create-stub-instance.js index 23baf4fb0..e4fd4148e 100644 --- a/lib/sinon/create-stub-instance.js +++ b/lib/sinon/create-stub-instance.js @@ -1,10 +1,11 @@ "use strict"; const stub = require("./stub"); +const sinonType = require("./util/core/sinon-type"); const forEach = require("@sinonjs/commons").prototypes.array.forEach; function isStub(value) { - return value && value.throws && value.returns; + return sinonType.get(value) === "stub"; } module.exports = function createStubInstance(constructor, overrides) { @@ -13,7 +14,7 @@ module.exports = function createStubInstance(constructor, overrides) { } const stubInstance = Object.create(constructor.prototype); - stubInstance[Symbol.for("SinonType")] = "StubInstance"; + sinonType.set(stubInstance, "stub-instance"); const stubbedObject = stub(stubInstance); diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 0edc88d5a..d521b6ce8 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -12,6 +12,7 @@ var spy = require("./spy"); var extend = require("./util/core/extend"); var getPropertyDescriptor = require("./util/core/get-property-descriptor"); var isEsModule = require("./util/core/is-es-module"); +var sinonType = require("./util/core/sinon-type"); var wrapMethod = require("./util/core/wrap-method"); var throwOnFalsyObject = require("./throw-on-falsy-object"); var valueToString = require("@sinonjs/commons").valueToString; @@ -58,6 +59,8 @@ function createStub(originalFunc) { id: `stub#${uuid++}`, }); + sinonType.set(proxy, "stub"); + return proxy; } diff --git a/lib/sinon/util/core/sinon-type.js b/lib/sinon/util/core/sinon-type.js new file mode 100644 index 000000000..3a674a2be --- /dev/null +++ b/lib/sinon/util/core/sinon-type.js @@ -0,0 +1,22 @@ +"use strict"; + +const sinonTypeSymbolProperty = Symbol("SinonType"); + +module.exports = { + /** + * Set the type of a Sinon object to make it possible to identify it later at runtime + * + * @param {object|Function} object object/function to set the type on + * @param {string} type the named type of the object/function + */ + set(object, type) { + Object.defineProperty(object, sinonTypeSymbolProperty, { + value: type, + configurable: false, + enumerable: false, + }); + }, + get(object) { + return object && object[sinonTypeSymbolProperty]; + }, +}; diff --git a/lib/sinon/util/core/wrap-method.js b/lib/sinon/util/core/wrap-method.js index 99ddd47a2..92036c247 100644 --- a/lib/sinon/util/core/wrap-method.js +++ b/lib/sinon/util/core/wrap-method.js @@ -4,6 +4,7 @@ const noop = () => {}; var getPropertyDescriptor = require("./get-property-descriptor"); var extend = require("./extend"); +const sinonType = require("./sinon-type"); var hasOwnProperty = require("@sinonjs/commons").prototypes.object.hasOwnProperty; var valueToString = require("@sinonjs/commons").valueToString; @@ -232,7 +233,7 @@ module.exports = function wrapMethod(object, property, method) { } } } - if (object[Symbol.for("SinonType")] === "StubInstance") { + if (sinonType.get(object) === "stub-instance") { // this is simply to avoid errors after restoring if something should // traverse the object in a cleanup phase, ref #2477 object[property] = noop; diff --git a/test/create-stub-instance-test.js b/test/create-stub-instance-test.js index c7db51ce9..6d08361e9 100644 --- a/test/create-stub-instance-test.js +++ b/test/create-stub-instance-test.js @@ -2,7 +2,7 @@ var referee = require("@sinonjs/referee"); var createStub = require("../lib/sinon/stub"); -var createStubInstance = require("../lib/sinon/create-stub-instance") +var createStubInstance = require("../lib/sinon/create-stub-instance"); var assert = referee.assert; var refute = referee.refute; @@ -155,4 +155,4 @@ describe("createStubInstance", function () { { message: "Cannot stub foo. Property does not exist!" } ); }); -}) +}); diff --git a/test/stub-test.js b/test/stub-test.js index 6524a3822..af880feae 100644 --- a/test/stub-test.js +++ b/test/stub-test.js @@ -2,7 +2,6 @@ var referee = require("@sinonjs/referee"); var createStub = require("../lib/sinon/stub"); -var createStubInstance = require("../lib/sinon/create-stub-instance") var createSpy = require("../lib/sinon/spy"); var createProxy = require("../lib/sinon/proxy"); var match = require("@sinonjs/samsam").createMatcher; @@ -3139,157 +3138,6 @@ describe("stub", function () { }); }); - describe(".createStubInstance", function () { - it("stubs existing methods", function () { - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - - var stub = createStubInstance(Class); - stub.method.returns(3); - assert.equals(3, stub.method()); - }); - - it("throws with no methods to stub", function () { - var Class = function () { - return; - }; - - assert.exception( - function () { - createStubInstance(Class); - }, - { - message: - "Found no methods on object to which we could apply mutations", - } - ); - }); - - it("doesn't call the constructor", function () { - var Class = function (a, b) { - var c = a + b; - throw c; - }; - Class.prototype.method = function () { - return; - }; - - var stub = createStubInstance(Class); - refute.exception(function () { - stub.method(3); - }); - }); - - it("retains non function values", function () { - var TYPE = "some-value"; - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - Class.prototype.type = TYPE; - - var stub = createStubInstance(Class); - assert.equals(TYPE, stub.type); - }); - - it("has no side effects on the prototype", function () { - var proto = { - method: function () { - throw new Error("error"); - }, - }; - var Class = function () { - return; - }; - Class.prototype = proto; - - var stub = createStubInstance(Class); - refute.exception(stub.method); - assert.exception(proto.method); - }); - - it("throws exception for non function params", function () { - var types = [{}, 3, "hi!"]; - - for (var i = 0; i < types.length; i++) { - // yes, it's silly to create functions in a loop, it's also a test - // eslint-disable-next-line no-loop-func - assert.exception(function () { - createStubInstance(types[i]); - }); - } - }); - - it("allows providing optional overrides", function () { - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - - var stub = createStubInstance(Class, { - method: createStub().returns(3), - }); - - assert.equals(3, stub.method()); - }); - - it("allows providing optional returned values", function () { - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - - var stub = createStubInstance(Class, { - method: 3, - }); - - assert.equals(3, stub.method()); - }); - - it("allows providing null as a return value", function () { - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - - var stub = createStubInstance(Class, { - method: null, - }); - - assert.equals(null, stub.method()); - }); - - it("throws an exception when trying to override non-existing property", function () { - var Class = function () { - return; - }; - Class.prototype.method = function () { - return; - }; - - assert.exception( - function () { - createStubInstance(Class, { - foo: createStub().returns(3), - }); - }, - { message: "Cannot stub foo. Property does not exist!" } - ); - }); - }); - describe(".callThrough", function () { it("does not call original function when arguments match conditional stub", function () { // We need a function here because we can't wrap properties that are already stubs