From ec4d592ee4faf87d7e592c4b99b3e6fec99105c8 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Thu, 25 Apr 2024 14:30:41 +0200 Subject: [PATCH] fix #2589: avoid invoking getter as side-effect (#2592) of creating restorerer function. Check for the odd case where we actually force setting a value through accessor functions and only look up the value at that time. The alternative would be more elaborate, check if the descriptor had a `get` or a `value` field, and then decide, but this should do the trick. --- lib/sinon/sandbox.js | 2 +- test/sandbox-test.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js index 6bab45f96..e6d8a0274 100644 --- a/lib/sinon/sandbox.js +++ b/lib/sinon/sandbox.js @@ -243,7 +243,7 @@ function Sandbox(opts = {}) { */ function getFakeRestorer(object, property, forceAssignment = false) { const descriptor = getPropertyDescriptor(object, property); - const value = object[property]; + const value = forceAssignment && object[property]; function restorer() { if (forceAssignment) { diff --git a/test/sandbox-test.js b/test/sandbox-test.js index cc1818015..c3c507359 100644 --- a/test/sandbox-test.js +++ b/test/sandbox-test.js @@ -1296,6 +1296,20 @@ describe("Sandbox", function () { }, ); }); + + it("do not call getter whilst replacing getter", function () { + const sandbox = this.sandbox; + const fake = sandbox.fake.returns("bar"); + const object = { + get foo() { + return fake(); + }, + }; + + sandbox.replaceGetter(object, "foo", () => "replacement"); + + refute(fake.called); + }); }); describe(".replaceSetter", function () {