Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable use of assignment in the presence of accessors #2537

Closed
wants to merge 2 commits into from
Closed

Enable use of assignment in the presence of accessors #2537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
81d977a
Make it possible to forceAssignment
fatso83 Aug 11, 2023
91036eb
Document the new feature
fatso83 Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/release-source/release/sandbox.md
View file
Open in desktop
Expand Up @@ -181,13 +181,13 @@ A convenience reference for [`sinon.assert`](./assertions)

_Since `sinon@2.0.0`_

#### `sandbox.replace(object, property, replacement);`
#### `sandbox.replace(object, property, replacement, options);`

Replaces `property` on `object` with `replacement` argument. Attempts to replace an already replaced value cause an exception. Returns the `replacement`.

`replacement` can be any value, including `spies`, `stubs` and `fakes`.

This method only works on non-accessor properties, for replacing accessors, use `sandbox.replaceGetter()` and `sandbox.replaceSetter()`.
By default, this method only works on non-accessor properties, for replacing accessors, use `sandbox.replaceGetter()` and `sandbox.replaceSetter()`.

```js
var myObject = {
Expand All @@ -204,6 +204,8 @@ console.log(myObject.myMethod());
// strawberry
```

You _can_ pass the option `{forceAssignment: boolean}` to avoid throwing on encountering a setter. This will pass the replacement into setter function and vice-versa use the getter to get the value used for restoring later on. One use case can be to do ESM module stubbing without resorting to external machinery such as module loaders (see [#2403](https://github.com/sinonjs/sinon/issues/2403)).

#### `sandbox.replaceGetter();`

Replaces getter for `property` on `object` with `replacement` argument. Attempts to replace an already replaced getter cause an exception.
Expand Down
35 changes: 25 additions & 10 deletions lib/sinon/sandbox.js
View file
Open in desktop
Expand Up @@ -208,11 +208,14 @@ function Sandbox() {
sandbox.injectedKeys.length = 0;
};

function getFakeRestorer(object, property) {
function getFakeRestorer(object, property, forceAssignment) {
const descriptor = getPropertyDescriptor(object, property);
const value = object[property];

function restorer() {
if (descriptor.isOwn) {
if (forceAssignment) {
object[property] = value;
} else if (descriptor.isOwn) {
Object.defineProperty(object, property, descriptor);
} else {
delete object[property];
Expand All @@ -237,7 +240,17 @@ function Sandbox() {
});
}

sandbox.replace = function replace(object, property, replacement) {
/**
* Replace an existing property
*
* @param {object|Function} object
* @param {string} property
* @param {*} replacement a fake, stub, spy or any other value
* @param {object} [options]
* @param {boolean} options.forceAssignment allows you to force use of assignment in the presence of a setter
* @returns {*}
*/
sandbox.replace = function replace(object, property, replacement, options) {
const descriptor = getPropertyDescriptor(object, property);

if (typeof descriptor === "undefined") {
Expand All @@ -252,26 +265,28 @@ function Sandbox() {
throw new TypeError("Expected replacement argument to be defined");
}

if (typeof descriptor.get === "function") {
throw new Error("Use sandbox.replaceGetter for replacing getters");
}
if (!options?.forceAssignment) {
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 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}`
]} with ${typeof replacement}`
);
}

verifyNotReplaced(object, property);

// store a function for restoring the replaced property
push(fakeRestorers, getFakeRestorer(object, property));
push(fakeRestorers, getFakeRestorer(object, property, options?.forceAssignment));

object[property] = replacement;

Expand Down
72 changes: 50 additions & 22 deletions test/sandbox-test.js
View file
Open in desktop
Expand Up @@ -52,7 +52,7 @@ describe("Sandbox", function () {
});

it("can be reset without failing when pre-configured to use a fake server", function () {
const sandbox = createSandbox({ useFakeServer: true });
const sandbox = createSandbox({useFakeServer: true});
refute.exception(function () {
sandbox.reset();
});
Expand Down Expand Up @@ -387,7 +387,7 @@ describe("Sandbox", function () {
foo: sinonStub().returns(3),
});
},
{ message: "Cannot stub foo. Property does not exist!" }
{message: "Cannot stub foo. Property does not exist!"}
);
});

Expand All @@ -398,6 +398,7 @@ describe("Sandbox", function () {
constructor() {
this.privateGetter = () => 42;
}

getValue() {
return this.privateGetter();
}
Expand Down Expand Up @@ -563,7 +564,7 @@ describe("Sandbox", function () {

describe("stub anything", function () {
beforeEach(function () {
this.object = { property: 42 };
this.object = {property: 42};
this.sandbox = new Sandbox();
});

Expand Down Expand Up @@ -910,7 +911,7 @@ describe("Sandbox", function () {
function () {
sandbox.replace(object, "property", replacement);
},
{ message: "Cannot replace string with function" }
{message: "Cannot replace string with function"}
);
});

Expand All @@ -927,7 +928,7 @@ describe("Sandbox", function () {
function () {
sandbox.replace(object, "property", replacement);
},
{ message: "Cannot replace function with string" }
{message: "Cannot replace function with string"}
);
});

Expand Down Expand Up @@ -987,8 +988,8 @@ describe("Sandbox", function () {
assert.equals(actual, replacement);
});

describe("when asked to replace a getter", function () {
it("should throw an Error", function () {
describe("when asked to replace a property with a getter", function () {
it("should throw an Error by default that informs of replaceGetter", function () {
const sandbox = this.sandbox;
const object = {
get foo() {
Expand All @@ -1008,7 +1009,7 @@ describe("Sandbox", function () {
});
});

describe("when asked to replace a setter", function () {
describe("when asked to replace a property with a setter", function () {
it("should throw an Error", function () {
const sandbox = this.sandbox;
const object = {
Expand All @@ -1028,6 +1029,26 @@ describe("Sandbox", function () {
}
);
});

it("should allow forcing assignment", 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;
},
};

assert.equals(object.foo(),'original')
sandbox.replace(object, "foo", sinonFake.returns('fake'), {forceAssignment: true});
assert.equals(object.foo(),'fake')
sandbox.restore()
assert.equals(object.foo(),'original');
})
});
});

Expand Down Expand Up @@ -1409,8 +1430,8 @@ describe("Sandbox", function () {
const sandbox = (this.sandbox = createSandbox());
const fakes = sandbox.getFakes();

fakes.push({ resetBehavior: sinonSpy() });
fakes.push({ resetBehavior: sinonSpy() });
fakes.push({resetBehavior: sinonSpy()});
fakes.push({resetBehavior: sinonSpy()});
});

it("calls resetBehavior on all fakes", function () {
Expand Down Expand Up @@ -1496,13 +1517,13 @@ describe("Sandbox", function () {
"useFakeTimers"
).returns({});

this.sandbox.useFakeTimers({ toFake: ["Date", "setTimeout"] });
this.sandbox.useFakeTimers({toFake: ["Date", "setTimeout"]});
this.sandbox.useFakeTimers({
toFake: ["setTimeout", "clearTimeout", "setInterval"],
});

assert(
useFakeTimersStub.calledWith({ toFake: ["Date", "setTimeout"] })
useFakeTimersStub.calledWith({toFake: ["Date", "setTimeout"]})
);
assert(
useFakeTimersStub.calledWith({
Expand Down Expand Up @@ -1772,8 +1793,14 @@ describe("Sandbox", function () {
/* eslint-disable no-empty-function, accessor-pairs */
this.sandbox.inject(this.obj);

const myObj = { a: function () {} };
function MyClass() {}
const myObj = {
a: function () {
}
};

function MyClass() {
}

MyClass.prototype.method1 = noop;
Object.defineProperty(myObj, "b", {
get: function () {
Expand All @@ -1782,7 +1809,8 @@ describe("Sandbox", function () {
configurable: true,
});
Object.defineProperty(myObj, "c", {
set: function () {},
set: function () {
},
configurable: true,
});

Expand Down Expand Up @@ -1868,8 +1896,8 @@ describe("Sandbox", function () {
const sandbox = createSandbox();
const fakes = sandbox.getFakes();

fakes.push({ verify: sinonSpy() });
fakes.push({ verify: sinonSpy() });
fakes.push({verify: sinonSpy()});
fakes.push({verify: sinonSpy()});

sandbox.verify();

Expand Down Expand Up @@ -1927,7 +1955,7 @@ describe("Sandbox", function () {
describe("configurable sandbox", function () {
beforeEach(function () {
this.requests = [];
this.fakeServer = { requests: this.requests };
this.fakeServer = {requests: this.requests};

this.useFakeTimersSpy = sinonSpy(sinonClock, "useFakeTimers");
sinonStub(fakeServer, "create").returns(this.fakeServer);
Expand Down Expand Up @@ -1987,7 +2015,7 @@ describe("Sandbox", function () {
};
const clock = {};
const spy = false;
const object = { server: server, clock: clock, spy: spy };
const object = {server: server, clock: clock, spy: spy};
const sandbox = createSandbox(
sinonConfig({
properties: ["server", "clock", "spy"],
Expand Down Expand Up @@ -2114,7 +2142,7 @@ describe("Sandbox", function () {
const sandbox = createSandbox(
sinonConfig({
properties: ["clock"],
useFakeTimers: { toFake: ["Date", "setTimeout"] },
useFakeTimers: {toFake: ["Date", "setTimeout"]},
})
);

Expand Down Expand Up @@ -2266,14 +2294,14 @@ describe("Sandbox", function () {
function () {
sandboxA.assert.fail("Some message");
},
{ name: "CustomErrorA" }
{name: "CustomErrorA"}
);

assert.exception(
function () {
sandboxB.assert.fail("Some message");
},
{ name: "CustomErrorB" }
{name: "CustomErrorB"}
);

sandboxA.restore();
Expand Down