Move and reuse assertions

sinonjs · Oct 3, 2023 · 8aa1581 · 8aa1581
1 parent e71b0c9
commit 8aa1581
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 47 deletions.
diff --git a/lib/sinon/sandbox.js b/lib/sinon/sandbox.js
@@ -35,6 +35,40 @@ function applyOnEach(fakes, method) {
     });
 }
 
+function throwOnAccessors(descriptor) {
+    if (typeof descriptor.get === "function") {
+        throw new Error("Use sandbox.replaceGetter for replacing getters");
+    }
+
+    if (typeof descriptor.set === "function") {
+        throw new Error("Use sandbox.replaceSetter for replacing setters");
+    }
+}
+
+function verifySameType(object, property, replacement) {
+    if (typeof object[property] !== typeof replacement) {
+        throw new TypeError(
+            `Cannot replace ${typeof object[
+                property
+                ]} with ${typeof replacement}`
+        );
+    }
+}
+
+function checkForValidArguments(descriptor, property, replacement) {
+    if (typeof descriptor === "undefined") {
+        throw new TypeError(
+            `Cannot replace non-existent property ${valueToString(
+                property
+            )}. Perhaps you meant sandbox.define()?`
+        );
+    }
+
+    if (typeof replacement === "undefined") {
+        throw new TypeError("Expected replacement argument to be defined");
+    }
+}
+
 function Sandbox() {
     const sandbox = this;
     let fakeRestorers = [];
@@ -239,34 +273,9 @@ function Sandbox() {
      */
     sandbox.replace = function replace(object, property, replacement) {
         const descriptor = getPropertyDescriptor(object, property);
-
-        if (typeof descriptor === "undefined") {
-            throw new TypeError(
-                `Cannot replace non-existent property ${valueToString(
-                    property
-                )}. Perhaps you meant sandbox.define()?`
-            );
-        }
-
-        if (typeof replacement === "undefined") {
-            throw new TypeError("Expected replacement argument to be defined");
-        }
-
-        if (typeof descriptor.get === "function") {
-            throw new Error("Use sandbox.replaceGetter for replacing getters");
-        }
-
-        if (typeof descriptor.set === "function") {
-            throw new Error("Use sandbox.replaceSetter for replacing setters");
-        }
-
-        if (typeof object[property] !== typeof replacement) {
-            throw new TypeError(
-                `Cannot replace ${typeof object[
-                    property
-                ]} with ${typeof replacement}`
-            );
-        }
+        checkForValidArguments(descriptor, property, replacement);
+        throwOnAccessors(descriptor);
+        verifySameType(object, property, replacement);
 
         verifyNotReplaced(object, property);
 
@@ -282,6 +291,12 @@ function Sandbox() {
     };
 
     sandbox.replace.usingAccessor = function replaceUsingAccessor(object, property, replacement) {
+        const descriptor = getPropertyDescriptor(object, property);
+        // checkForValidArguments(descriptor, property, replacement);
+        // verifySameType(object, property, replacement);
+
+        verifyNotReplaced(object, property);
+
         // store a function for restoring the replaced property
         push(
             fakeRestorers,

diff --git a/test/sandbox-test.js b/test/sandbox-test.js
@@ -1114,27 +1114,29 @@ describe("Sandbox", function () {
                     }
                 );
             });
+        });
+    });
 
-            it("should allow using assignment when replacing a value", function () {
-                const sandbox = this.sandbox;
-                let hiddenFoo = () => "original";
-                const object = {
-                    // eslint-disable-next-line accessor-pairs
-                    get foo() {
-                        return hiddenFoo;
-                    },
-                    set foo(value) {
-                        hiddenFoo = value;
-                    },
-                };
+    describe('.replace.usingAccessor', function () {
+        it("should allow using assignment when replacing a value", function () {
+            const sandbox = createSandbox();
+            let quaziPrivateStateOfObject =  "original";
+            const object = {
+                // eslint-disable-next-line accessor-pairs
+                get foo() {
+                    return quaziPrivateStateOfObject;
+                },
+                set foo(value) {
+                    quaziPrivateStateOfObject = value;
+                },
+            };
 
-                assert.equals(object.foo(), "original");
-                sandbox.replace.usingAccessor(object, "foo", sinonFake.returns("fake"));
-                assert.equals(object.foo(), "fake");
-                sandbox.restore();
-                assert.equals(object.foo(), "original");
-            });
-        });
+            assert.equals(object.foo, "original");
+            sandbox.replace.usingAccessor(object, "foo", "fake");
+            assert.equals(object.foo, "fake");
+            sandbox.restore();
+            assert.equals(object.foo, "original");
+        })
     });
 
     describe(".replaceGetter", function () {