From 477064b628c65220ce9d0ac16cd33ab9b1da93da Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 26 Mar 2023 12:50:22 +0200 Subject: [PATCH] fix: make it possible to call through to underlying stub in stub instance (#2503) * fix: make it possible to call through to underlying stub in stub instances refs #2477 refs #2501 * internal: Extract underlying createStubInstance * internal: extract tests into own module * internal: extract sinon type checking into own module closes #2501 --- lib/sinon/create-stub-instance.js | 36 +++++++ lib/sinon/sandbox.js | 3 +- lib/sinon/stub.js | 32 +----- lib/sinon/util/core/sinon-type.js | 22 ++++ lib/sinon/util/core/wrap-method.js | 8 ++ test/create-stub-instance-test.js | 158 +++++++++++++++++++++++++++++ test/issues/issues-test.js | 17 ++++ test/stub-test.js | 152 --------------------------- 8 files changed, 246 insertions(+), 182 deletions(-) create mode 100644 lib/sinon/create-stub-instance.js create mode 100644 lib/sinon/util/core/sinon-type.js create mode 100644 test/create-stub-instance-test.js diff --git a/lib/sinon/create-stub-instance.js b/lib/sinon/create-stub-instance.js new file mode 100644 index 000000000..e4fd4148e --- /dev/null +++ b/lib/sinon/create-stub-instance.js @@ -0,0 +1,36 @@ +"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 sinonType.get(value) === "stub"; +} + +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); + sinonType.set(stubInstance, "stub-instance"); + + 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 8b9e32af7..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; } @@ -126,35 +129,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."); - } - - // eslint-disable-next-line no-empty-function - const noop = () => {}; - const defaultNoOpInstance = Object.create(constructor.prototype); - walkObject((obj, prop) => (obj[prop] = noop), defaultNoOpInstance); - - const stubbedObject = stub(defaultNoOpInstance); - - 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/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 fc2cd4815..92036c247 100644 --- a/lib/sinon/util/core/wrap-method.js +++ b/lib/sinon/util/core/wrap-method.js @@ -1,7 +1,10 @@ "use strict"; +// eslint-disable-next-line no-empty-function +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; @@ -230,6 +233,11 @@ module.exports = function wrapMethod(object, property, method) { } } } + 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; + } } return method; diff --git a/test/create-stub-instance-test.js b/test/create-stub-instance-test.js new file mode 100644 index 000000000..6d08361e9 --- /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!" } + ); + }); +}); 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); + }); + }); }); diff --git a/test/stub-test.js b/test/stub-test.js index 2537a2f61..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/stub").createStubInstance; 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