Skip to content

Commit

Permalink
Remove support for stubbing undefined props (#1557)
Browse files Browse the repository at this point in the history
This was a longer discussion that can be found in the
issues #1508, #1552 and #1537 - and internally amongst the core team.

Support for stubbing props using get()/set() was added in #1237
and also worked for undefined properties.

This type of behavior (stubbing undefined props) has been
explicitly disabled for sandboxes, so in order for Sinon to
behave in a consistent way, we should do the same here.

This change will bring normal stubs back to how it has
used to be historically, and will make sandbox stubs
and normal sinon stubs behave the same.

It might break things for people relying on the behavior
that has been present since Sinon 2.0, but it should
make things more reliable going forward.
  • Loading branch information
fatso83 committed Sep 18, 2017
1 parent e9f40a2 commit 34fc2ba
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 58 deletions.
5 changes: 5 additions & 0 deletions lib/sinon/stub.js
Expand Up @@ -9,6 +9,7 @@ var getPropertyDescriptor = require("./util/core/get-property-descriptor");
var wrapMethod = require("./util/core/wrap-method");
var stubEntireObject = require("./stub-entire-object");
var throwOnFalsyObject = require("./throw-on-falsy-object");
var valueToString = require("./util/core/value-to-string");

var slice = Array.prototype.slice;

Expand All @@ -19,6 +20,10 @@ function stub(object, property) {

throwOnFalsyObject.apply(null, arguments);

if (object && typeof property !== "undefined" && !(property in object)) {
throw new TypeError("Cannot stub non-existent own property " + valueToString(property));
}

var actualDescriptor = getPropertyDescriptor(object, property);
var isStubbingEntireObject = typeof property === "undefined" && typeof object === "object";
var isCreatingNewStub = !object && typeof property === "undefined";
Expand Down
68 changes: 10 additions & 58 deletions test/stub-test.js
Expand Up @@ -5,7 +5,6 @@ var createStub = require("../lib/sinon/stub");
var createStubInstance = require("../lib/sinon/stub").createStubInstance;
var createSpy = require("../lib/sinon/spy");
var sinonMatch = require("../lib/sinon/match");
var getPropertyDescriptor = require("../lib/sinon/util/core/get-property-descriptor");
var deprecated = require("../lib/sinon/util/core/deprecated");
var assert = referee.assert;
var refute = referee.refute;
Expand Down Expand Up @@ -1164,6 +1163,16 @@ describe("stub", function () {

stub.restore();
});

it("throws if stubbing non-existent property", function () {
var myObj = {};

assert.exception(function () {
createStub(myObj, "ouch");
});

refute.defined(myObj.ouch);
});
});

describe("stubbed function", function () {
Expand Down Expand Up @@ -2579,16 +2588,6 @@ describe("stub", function () {
assert.equals(myObj.prop, "bar");
});

it("can set getters for non-existing properties", function () {
var myObj = {};

createStub(myObj, "prop").get(function getterFn() {
return "bar";
});

assert.equals(myObj.prop, "bar");
});

it("can restore stubbed setters for functions", function () {
var propFn = function propFn() {
return "bar";
Expand Down Expand Up @@ -2627,19 +2626,6 @@ describe("stub", function () {
assert.equals(myObj.prop, "bar");
});

it("can restore stubbed getters for previously undefined properties", function () {
var myObj = {};

var stub = createStub(myObj, "nonExisting");

stub.get(function getterFn() {
return "baz";
});

stub.restore();

assert.equals(getPropertyDescriptor(myObj, "nonExisting"), undefined);
});
});

describe(".set", function () {
Expand Down Expand Up @@ -2689,18 +2675,6 @@ describe("stub", function () {
assert.equals(myObj.example, "bar");
});

it("can set setters for non-existing properties", function () {
var myObj = {};

createStub(myObj, "prop").set(function setterFn() {
myObj.example = "bar";
});

myObj.prop = "foo";

assert.equals(myObj.example, "bar");
});

it("can restore stubbed setters for functions", function () {
var propFn = function propFn() {
return "bar";
Expand Down Expand Up @@ -2741,19 +2715,6 @@ describe("stub", function () {
assert.equals(myObj.otherProp, "bar");
});

it("can restore stubbed setters for previously undefined properties", function () {
var myObj = {};

var stub = createStub(myObj, "nonExisting");

stub.set(function setterFn() {
myObj.otherProp = "baz";
});

stub.restore();

assert.equals(getPropertyDescriptor(myObj, "nonExisting"), undefined);
});
});

describe(".value", function () {
Expand All @@ -2777,15 +2738,6 @@ describe("stub", function () {
assert.equals(myObj.prop, "rawString");
});

it("allows restoring previously undefined properties", function () {
var obj = {};
var stub = createStub(obj, "nonExisting").value(2);

stub.restore();

assert.equals(getPropertyDescriptor(obj, "nonExisting"), undefined);
});

it("allows stubbing function static properties", function () {
var myFunc = function () {};
myFunc.prop = "rawString";
Expand Down

0 comments on commit 34fc2ba

Please sign in to comment.