From 5eb187c451a5e2da996b54d721a455a4c584565c Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sat, 28 Oct 2023 21:23:38 +0200 Subject: [PATCH 1/4] breaking: Remove sinon.defaultConfig and related modules default-config and get-config are leftovers from when Sinon shipped with sinon.test (now the independent NPM module 'sinon-test'). These serve no purpose internally, and really have no purpose but to help sinon-test create a base default. If needed, these can be copied into the sinon-test project. No projects should depend on these (my assumption), but since it is a change of the API we mark it as a breaking change fixes #2561 --- lib/create-sinon-api.js | 1 - lib/sinon/util/core/default-config.js | 21 ----- test/get-config.js | 21 ----- test/sandbox-test.js | 115 ++++++++++---------------- test/util/core/get-config-test.js | 34 -------- 5 files changed, 45 insertions(+), 147 deletions(-) delete mode 100644 lib/sinon/util/core/default-config.js delete mode 100644 test/get-config.js delete mode 100644 test/util/core/get-config-test.js diff --git a/lib/create-sinon-api.js b/lib/create-sinon-api.js index 9ef9ec126..58f0f93d1 100644 --- a/lib/create-sinon-api.js +++ b/lib/create-sinon-api.js @@ -25,7 +25,6 @@ module.exports = function createApi(opts = { sinonXhrLib: nise }) { restoreObject: require("./sinon/restore-object"), expectation: require("./sinon/mock-expectation"), - defaultConfig: require("./sinon/util/core/default-config"), // fake timers timers: fakeTimers.timers, diff --git a/lib/sinon/util/core/default-config.js b/lib/sinon/util/core/default-config.js deleted file mode 100644 index 60170b442..000000000 --- a/lib/sinon/util/core/default-config.js +++ /dev/null @@ -1,21 +0,0 @@ -"use strict"; - -module.exports = { - injectInto: null, - properties: [ - "spy", - "stub", - "mock", - "clock", - "server", - "requests", - "fake", - "define", - "replace", - "replaceSetter", - "replaceGetter", - "createStubInstance", - ], - useFakeTimers: true, - useFakeServer: true, -}; diff --git a/test/get-config.js b/test/get-config.js deleted file mode 100644 index 168af9069..000000000 --- a/test/get-config.js +++ /dev/null @@ -1,21 +0,0 @@ -"use strict"; - -const defaultConfig = require("../lib/sinon/util/core/default-config"); -const hasOwnProperty = - require("@sinonjs/commons").prototypes.object.hasOwnProperty; - -module.exports = function getConfig(custom) { - const config = {}; - let prop; - const kustom = custom || {}; - - for (prop in defaultConfig) { - if (hasOwnProperty(defaultConfig, prop)) { - config[prop] = hasOwnProperty(kustom, prop) - ? kustom[prop] - : defaultConfig[prop]; - } - } - - return config; -}; diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 28b8f77c7..7b5de178e 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -14,7 +14,6 @@ const createSandbox = require("../lib/sinon/create-sandbox"); const sinonFake = require("../lib/sinon/fake"); const sinonSpy = require("../lib/sinon/spy"); const sinonStub = require("../lib/sinon/stub"); -const sinonConfig = require("./get-config"); const sinonClock = require("../lib/sinon/util/fake-timers"); const supportsAjax = @@ -2053,11 +2052,9 @@ describe("Sandbox", function () { }); it("yields stub, mock as arguments", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["stub", "mock"], - }), - ); + const sandbox = createSandbox({ + properties: ["stub", "mock"], + }); assert.equals(sandbox.args.length, 2); assert.stub(sandbox.args[0]()); @@ -2067,11 +2064,9 @@ describe("Sandbox", function () { }); it("yields spy, stub, mock as arguments", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["spy", "stub", "mock"], - }), - ); + const sandbox = createSandbox({ + properties: ["spy", "stub", "mock"], + }); assert.spy(sandbox.args[0]()); assert.stub(sandbox.args[1]()); @@ -2081,12 +2076,10 @@ describe("Sandbox", function () { }); it("does not yield server when not faking xhr", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["server", "stub", "mock"], - useFakeServer: false, - }), - ); + const sandbox = createSandbox({ + properties: ["server", "stub", "mock"], + useFakeServer: false, + }); assert.equals(sandbox.args.length, 2); assert.stub(sandbox.args[0]()); @@ -2102,12 +2095,10 @@ describe("Sandbox", function () { const clock = {}; const spy = false; const object = { server: server, clock: clock, spy: spy }; - const sandbox = createSandbox( - sinonConfig({ - properties: ["server", "clock", "spy"], - injectInto: object, - }), - ); + const sandbox = createSandbox({ + properties: ["server", "clock", "spy"], + injectInto: object, + }); assert.same(object.server, server); assert.same(object.clock, clock); @@ -2119,11 +2110,9 @@ describe("Sandbox", function () { if (supportsAjax) { describe("ajax options", function () { it("yields server when faking xhr", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["server", "stub", "mock"], - }), - ); + const sandbox = createSandbox({ + properties: ["server", "stub", "mock"], + }); assert.equals(sandbox.args.length, 3); assert.equals(sandbox.args[0], this.fakeServer); @@ -2134,12 +2123,10 @@ describe("Sandbox", function () { }); it("uses serverWithClock when faking xhr", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["server"], - useFakeServer: fakeServerWithClock, - }), - ); + const sandbox = createSandbox({ + properties: ["server"], + useFakeServer: fakeServerWithClock, + }); assert.fakeServerWithClock( sandbox.args[0], @@ -2164,11 +2151,9 @@ describe("Sandbox", function () { }); it("yields clock when faking timers", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["server", "clock"], - }), - ); + const sandbox = createSandbox({ + properties: ["server", "clock"], + }); assert.same(sandbox.args[0], this.fakeServer); assert.clock(sandbox.args[1]); @@ -2179,12 +2164,10 @@ describe("Sandbox", function () { it("injects properties into object", function () { const object = {}; - const sandbox = createSandbox( - sinonConfig({ - properties: ["server", "clock"], - injectInto: object, - }), - ); + const sandbox = createSandbox({ + properties: ["server", "clock"], + injectInto: object, + }); assert.equals(sandbox.args.length, 0); assert.equals(object.server, this.fakeServer); @@ -2200,13 +2183,11 @@ describe("Sandbox", function () { it("should inject server and clock when only enabling them", function () { const object = {}; - const sandbox = createSandbox( - sinonConfig({ - injectInto: object, - useFakeTimers: true, - useFakeServer: true, - }), - ); + const sandbox = createSandbox({ + injectInto: object, + useFakeTimers: true, + useFakeServer: true, + }); assert.equals(sandbox.args.length, 0); assert.equals(object.server, this.fakeServer); @@ -2225,12 +2206,10 @@ describe("Sandbox", function () { // This is currently testing the internals of useFakeTimers, we could possibly change it to be based on // behavior. it("fakes specified timers", function () { - const sandbox = createSandbox( - sinonConfig({ - properties: ["clock"], - useFakeTimers: { toFake: ["Date", "setTimeout"] }, - }), - ); + const sandbox = createSandbox({ + properties: ["clock"], + useFakeTimers: { toFake: ["Date", "setTimeout"] }, + }); assert( this.useFakeTimersSpy.calledWith({ @@ -2244,12 +2223,10 @@ describe("Sandbox", function () { it("injects sandbox", function () { const object = {}; - const sandbox = createSandbox( - sinonConfig({ - properties: ["sandbox", "spy"], - injectInto: object, - }), - ); + const sandbox = createSandbox({ + properties: ["sandbox", "spy"], + injectInto: object, + }); assert.equals(sandbox.args.length, 0); assert.isFunction(object.spy); @@ -2261,12 +2238,10 @@ describe("Sandbox", function () { it("injects match", function () { const object = {}; - const sandbox = createSandbox( - sinonConfig({ - properties: ["match"], - injectInto: object, - }), - ); + const sandbox = createSandbox({ + properties: ["match"], + injectInto: object, + }); assert.same(object.match, match); diff --git a/test/util/core/get-config-test.js b/test/util/core/get-config-test.js deleted file mode 100644 index 3586d3d05..000000000 --- a/test/util/core/get-config-test.js +++ /dev/null @@ -1,34 +0,0 @@ -"use strict"; - -const referee = require("@sinonjs/referee"); -const getConfig = require("../../get-config"); -const defaultConfig = require("../../../lib/sinon/util/core/default-config"); -const assert = referee.assert; -const refute = referee.refute; - -describe("core/util/getConfig", function () { - describe(".getConfig", function () { - it("gets copy of default config", function () { - const config = getConfig(); - - refute.same(config, defaultConfig); - assert.equals(config.injectInto, defaultConfig.injectInto); - assert.equals(config.properties, defaultConfig.properties); - assert.equals(config.useFakeTimers, defaultConfig.useFakeTimers); - assert.equals(config.useFakeServer, defaultConfig.useFakeServer); - }); - - it("should override specified properties", function () { - const config = getConfig({ - properties: ["stub", "mock"], - useFakeServer: false, - }); - - refute.same(config, defaultConfig); - assert.equals(config.injectInto, defaultConfig.injectInto); - assert.equals(config.properties, ["stub", "mock"]); - assert.equals(config.useFakeTimers, defaultConfig.useFakeTimers); - assert.isFalse(config.useFakeServer); - }); - }); -}); From 023b62cb7637a295e85dbc91262d3d52776e4299 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 29 Oct 2023 06:38:41 +0100 Subject: [PATCH 2/4] fixed up tests that were lying It seemed like the the 'injectInto' option would expose most props by default. This was not the case. That was formerly hidden by using the getConfig call that added props that were never used in the actual implementation. Added another test to make this more explicit. Will add docs on this. --- test/sandbox-test.js | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/test/sandbox-test.js b/test/sandbox-test.js index 7b5de178e..cc1818015 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -2111,6 +2111,7 @@ describe("Sandbox", function () { describe("ajax options", function () { it("yields server when faking xhr", function () { const sandbox = createSandbox({ + useFakeServer: true, properties: ["server", "stub", "mock"], }); @@ -2153,6 +2154,8 @@ describe("Sandbox", function () { it("yields clock when faking timers", function () { const sandbox = createSandbox({ properties: ["server", "clock"], + useFakeServer: true, + useFakeTimers: true, }); assert.same(sandbox.args[0], this.fakeServer); @@ -2161,42 +2164,37 @@ describe("Sandbox", function () { sandbox.restore(); }); - it("injects properties into object", function () { + it("should inject server and clock when enabling them", function () { const object = {}; const sandbox = createSandbox({ - properties: ["server", "clock"], injectInto: object, + properties: ["clock", "server", "requests"], + useFakeTimers: true, + useFakeServer: true, }); assert.equals(sandbox.args.length, 0); assert.equals(object.server, this.fakeServer); assert.clock(object.clock); - assert.isUndefined(object.spy); - assert.isUndefined(object.stub); - assert.isUndefined(object.mock); - assert.isUndefined(object.requests); + assert.isArray(object.requests); sandbox.restore(); }); - it("should inject server and clock when only enabling them", function () { + it("should not inject server and clock if not enabled, even if the props are whitelisted", function () { const object = {}; const sandbox = createSandbox({ injectInto: object, - useFakeTimers: true, - useFakeServer: true, + properties: ["clock", "server", "requests"], + useFakeTimers: false, + useFakeServer: false, }); - assert.equals(sandbox.args.length, 0); - assert.equals(object.server, this.fakeServer); - assert.clock(object.clock); - assert.isFunction(object.spy); - assert.isFunction(object.stub); - assert.isFunction(object.mock); - assert.isArray(object.requests); + assert.isUndefined(object.requests); assert.isUndefined(object.sandbox); + assert.isUndefined(object.clock); sandbox.restore(); }); @@ -2247,6 +2245,16 @@ describe("Sandbox", function () { sandbox.restore(); }); + + it("does not inject any properties by default", function () { + const object = {}; + + createSandbox({ + injectInto: object, + }); + + assert.equals(Object.keys(object), []); + }); }); describe("getters and setters", function () { From e3e6f3780dedc9efa74bb8e4403d229443b691b5 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 29 Oct 2023 06:49:46 +0100 Subject: [PATCH 3/4] Document Sandbox#inject This was added in Sinon 0.6 but has never been documented --- docs/release-source/release/sandbox.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/release-source/release/sandbox.md b/docs/release-source/release/sandbox.md index 7847a688b..28961e1b0 100644 --- a/docs/release-source/release/sandbox.md +++ b/docs/release-source/release/sandbox.md @@ -119,7 +119,11 @@ and overflow your display. ```
-``` + +#### `inject(facadeObject)` + +This is injects all the properties of the sandbox into the facade object. +This is equivalent to specifying all the available properties in `properties` when you create a sandbox with `injectInto`. ##### `useFakeTimers` @@ -156,6 +160,12 @@ const sandbox = sinon.createSandbox({ }); ``` +Alternatively you can use the `sandbox.inject({})` method, which will inject all the sandbox methods by default, which is _usually_ what you want. + +```javascript +const myFacade = sandbox.inject({}); +``` + #### `sandbox.assert();` A convenience reference for [`sinon.assert`](./assertions) From 94ee6d3731754fd55d57903018ce8016a33fa5d2 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 29 Oct 2023 06:51:33 +0100 Subject: [PATCH 4/4] Note that 'properties' is empty by default --- docs/release-source/release/sandbox.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/release-source/release/sandbox.md b/docs/release-source/release/sandbox.md index 28961e1b0..f4d3c56d3 100644 --- a/docs/release-source/release/sandbox.md +++ b/docs/release-source/release/sandbox.md @@ -79,11 +79,13 @@ const sandbox = sinon.createSandbox({ ##### `injectInto` The sandbox's methods can be injected into another object for convenience. The -`injectInto` configuration option can name an object to add properties to. See the example further down the page. +`injectInto` configuration option can name an object to add properties to. Note that you explicitly need to specify all the properties you want to expose using the `properties` field. + +See the example further down the page. ##### `properties` -Which properties to inject into the facade object. Note that only naming "server" here is not sufficient to have a `server` property show up in the target object, you also have to set `useFakeServer` to `true`. +Which properties to inject into the facade object. By default empty! Note that only naming "server" here is not sufficient to have a `server` property show up in the target object, you also have to set `useFakeServer` to `true`. The list of properties that can be injected are the ones exposed by the object returned by the function `inject`: