From b775f1b4174c5a92fa7fa8f70fbf3f4b5466a39e Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Sun, 26 Mar 2023 14:59:25 +0200 Subject: [PATCH] Avoid tampering with globals and other modules' exports in tests (#2504) * fix: do not modify the Function prototype when running tests Made a single test fail as the Function prototype had been mutated by a later test in the test suite. refs #2502 * fix: do not mutate matcher object message This made 3 of the assertion tests fail when re-running as Sinon's spy formatter changed the exports of the samsam module * Use latest version of samsam to get immutable messages Ensure we cannot do the same mistake again --- lib/sinon/spy-formatters.js | 5 +++-- package-lock.json | 32 +++++++++++++++++++++++++++----- package.json | 2 +- test/spy-formatters-test.js | 21 +++++++++++++++++++++ test/stub-test.js | 15 ++++++++------- 5 files changed, 60 insertions(+), 15 deletions(-) create mode 100644 test/spy-formatters-test.js diff --git a/lib/sinon/spy-formatters.js b/lib/sinon/spy-formatters.js index f2ddce139..5384e3c10 100644 --- a/lib/sinon/spy-formatters.js +++ b/lib/sinon/spy-formatters.js @@ -14,13 +14,14 @@ var slice = arrayProto.slice; function colorSinonMatchText(matcher, calledArg, calledArgMessage) { var calledArgumentMessage = calledArgMessage; + var matcherMessage = matcher.message; if (!matcher.test(calledArg)) { - matcher.message = color.red(matcher.message); + matcherMessage = color.red(matcher.message); if (calledArgumentMessage) { calledArgumentMessage = color.green(calledArgumentMessage); } } - return `${calledArgumentMessage} ${matcher.message}`; + return `${calledArgumentMessage} ${matcherMessage}`; } function colorDiffText(diff) { diff --git a/package-lock.json b/package-lock.json index 9717e491c..8a4e79027 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "dependencies": { "@sinonjs/commons": "^3.0.0", "@sinonjs/fake-timers": "^10.0.2", - "@sinonjs/samsam": "^7.0.1", + "@sinonjs/samsam": "^8.0.0", "diff": "^5.1.0", "nise": "^5.1.4", "supports-color": "^7.2.0" @@ -737,10 +737,21 @@ "type-detect": "4.0.8" } }, - "node_modules/@sinonjs/samsam": { + "node_modules/@sinonjs/referee/node_modules/@sinonjs/samsam": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-7.0.1.tgz", "integrity": "sha512-zsAk2Jkiq89mhZovB2LLOdTCxJF4hqqTToGP0ASWlhp4I1hqOjcfmZGafXntCN7MDC6yySH0mFHrYtHceOeLmw==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^2.0.0", + "lodash.get": "^4.4.2", + "type-detect": "^4.0.8" + } + }, + "node_modules/@sinonjs/samsam": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-8.0.0.tgz", + "integrity": "sha512-Bp8KUVlLp8ibJZrnvq2foVhP0IVX2CIprMJPK0vqGqgrDa0OHVKeZyBykqskkrdxV6yKBPmGasO8LVjAKR3Gew==", "dependencies": { "@sinonjs/commons": "^2.0.0", "lodash.get": "^4.4.2", @@ -9645,13 +9656,24 @@ "requires": { "type-detect": "4.0.8" } + }, + "@sinonjs/samsam": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-7.0.1.tgz", + "integrity": "sha512-zsAk2Jkiq89mhZovB2LLOdTCxJF4hqqTToGP0ASWlhp4I1hqOjcfmZGafXntCN7MDC6yySH0mFHrYtHceOeLmw==", + "dev": true, + "requires": { + "@sinonjs/commons": "^2.0.0", + "lodash.get": "^4.4.2", + "type-detect": "^4.0.8" + } } } }, "@sinonjs/samsam": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-7.0.1.tgz", - "integrity": "sha512-zsAk2Jkiq89mhZovB2LLOdTCxJF4hqqTToGP0ASWlhp4I1hqOjcfmZGafXntCN7MDC6yySH0mFHrYtHceOeLmw==", + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-8.0.0.tgz", + "integrity": "sha512-Bp8KUVlLp8ibJZrnvq2foVhP0IVX2CIprMJPK0vqGqgrDa0OHVKeZyBykqskkrdxV6yKBPmGasO8LVjAKR3Gew==", "requires": { "@sinonjs/commons": "^2.0.0", "lodash.get": "^4.4.2", diff --git a/package.json b/package.json index 38c4cdf92..4de48a2b1 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "dependencies": { "@sinonjs/commons": "^3.0.0", "@sinonjs/fake-timers": "^10.0.2", - "@sinonjs/samsam": "^7.0.1", + "@sinonjs/samsam": "^8.0.0", "diff": "^5.1.0", "nise": "^5.1.4", "supports-color": "^7.2.0" diff --git a/test/spy-formatters-test.js b/test/spy-formatters-test.js new file mode 100644 index 000000000..287dd1673 --- /dev/null +++ b/test/spy-formatters-test.js @@ -0,0 +1,21 @@ +"use strict"; +const { D } = require("./../lib/sinon/spy-formatters"); +const sinon = require("../lib/sinon"); +const { assert } = require("@sinonjs/referee"); + +describe('formatter specifier "D"', function () { + it("should not mutate matchers passed as arguments", function () { + const matcher = sinon.match(function test() { + return false; + }, "something"); + assert.equals(matcher.message, "something"); + + const stub = sinon.stub(); + + stub(1, 2, 3); + /* eslint-disable new-cap */ + D(stub, [matcher]); + + assert.equals(matcher.message, "something"); + }); +}); diff --git a/test/stub-test.js b/test/stub-test.js index af880feae..0e83dc76e 100644 --- a/test/stub-test.js +++ b/test/stub-test.js @@ -173,6 +173,7 @@ describe("stub", function () { test: func, }; func.aProp = 42; + createStub(object, "test"); assert.equals(object.test.aProp, 42); @@ -1535,16 +1536,16 @@ describe("stub", function () { }); it("stubs methods of function", function () { - var func = function () { - return; - }; + class FunctionType extends Function { + func2() { + return 42; + } + } + + var func = new FunctionType(); func.func1 = function () { return; }; - // eslint-disable-next-line no-proto - func.__proto__.func2 = function () { - return; - }; createStub(func);