Skip to content

Commit 00b56bf

Browse files
committed
fix: ensure last behavior setter always wins
Add resetReturnBehavior() to clear all mutually exclusive return-behavior flags before each setter runs, so the most recently called setter takes effect regardless of invoke priority order in behavior.invoke(). Add tests for previously broken combinations where a higher-priority flag persisted after a lower-priority setter was called.
1 parent 1381482 commit 00b56bf

2 files changed

Lines changed: 93 additions & 42 deletions

File tree

lib/sinon/default-behaviors.js

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,35 @@ const slice = arrayProto.slice;
1010
const useLeftMostCallback = -1;
1111
const useRightMostCallback = -2;
1212

13+
/**
14+
* Resets all mutually exclusive "return behavior" flags so that the most
15+
* recently called behavior setter always takes effect ("last call wins").
16+
*
17+
* Without this, flags like `returnArgAt` (set by `returnsArg()`) can silently
18+
* persist and override a later `.returns()` call because `returnArgAt` is
19+
* checked first in behavior.invoke().
20+
*
21+
* @param {object} fake - the stub behavior object
22+
*/
23+
function resetReturnBehavior(fake) {
24+
fake.returnArgAt = undefined;
25+
fake.returnThis = false;
26+
fake.throwArgAt = undefined;
27+
fake.fakeFn = undefined;
28+
fake.returnValue = undefined;
29+
fake.returnValueDefined = false;
30+
fake.resolve = false;
31+
fake.reject = false;
32+
fake.resolveArgAt = undefined;
33+
fake.resolveThis = false;
34+
fake.exception = undefined;
35+
fake.exceptionCreator = undefined;
36+
fake.callsThrough = false;
37+
fake.callsThroughWithNew = false;
38+
}
39+
1340
function throwsException(fake, error, message) {
41+
resetReturnBehavior(fake);
1442
if (typeof error === "function") {
1543
fake.exceptionCreator = error;
1644
} else if (typeof error === "string") {
@@ -32,10 +60,8 @@ function throwsException(fake, error, message) {
3260

3361
const defaultBehaviors = {
3462
callsFake: function callsFake(fake, fn) {
63+
resetReturnBehavior(fake);
3564
fake.fakeFn = fn;
36-
fake.exception = undefined;
37-
fake.exceptionCreator = undefined;
38-
fake.callsThrough = false;
3965
},
4066

4167
callsArg: function callsArg(fake, index) {
@@ -143,65 +169,46 @@ const defaultBehaviors = {
143169
throwsException: throwsException,
144170

145171
returns: function returns(fake, value) {
146-
fake.callsThrough = false;
172+
resetReturnBehavior(fake);
147173
fake.returnValue = value;
148-
fake.resolve = false;
149-
fake.reject = false;
150174
fake.returnValueDefined = true;
151-
fake.exception = undefined;
152-
fake.exceptionCreator = undefined;
153-
fake.fakeFn = undefined;
154175
},
155176

156177
returnsArg: function returnsArg(fake, index) {
157178
if (typeof index !== "number") {
158179
throw new TypeError("argument index is not number");
159180
}
160-
fake.callsThrough = false;
161-
181+
resetReturnBehavior(fake);
162182
fake.returnArgAt = index;
163183
},
164184

165185
throwsArg: function throwsArg(fake, index) {
166186
if (typeof index !== "number") {
167187
throw new TypeError("argument index is not number");
168188
}
169-
fake.callsThrough = false;
170-
189+
resetReturnBehavior(fake);
171190
fake.throwArgAt = index;
172191
},
173192

174193
returnsThis: function returnsThis(fake) {
194+
resetReturnBehavior(fake);
175195
fake.returnThis = true;
176-
fake.callsThrough = false;
177196
},
178197

179198
resolves: function resolves(fake, value) {
199+
resetReturnBehavior(fake);
180200
fake.returnValue = value;
181201
fake.resolve = true;
182-
fake.resolveThis = false;
183-
fake.reject = false;
184202
fake.returnValueDefined = true;
185-
fake.exception = undefined;
186-
fake.exceptionCreator = undefined;
187-
fake.fakeFn = undefined;
188-
fake.callsThrough = false;
189203
},
190204

191205
resolvesArg: function resolvesArg(fake, index) {
192206
if (typeof index !== "number") {
193207
throw new TypeError("argument index is not number");
194208
}
209+
resetReturnBehavior(fake);
195210
fake.resolveArgAt = index;
196-
fake.returnValue = undefined;
197211
fake.resolve = true;
198-
fake.resolveThis = false;
199-
fake.reject = false;
200-
fake.returnValueDefined = false;
201-
fake.exception = undefined;
202-
fake.exceptionCreator = undefined;
203-
fake.fakeFn = undefined;
204-
fake.callsThrough = false;
205212
},
206213

207214
rejects: function rejects(fake, error, message) {
@@ -214,29 +221,17 @@ const defaultBehaviors = {
214221
} else {
215222
reason = error;
216223
}
224+
resetReturnBehavior(fake);
217225
fake.returnValue = reason;
218-
fake.resolve = false;
219-
fake.resolveThis = false;
220226
fake.reject = true;
221227
fake.returnValueDefined = true;
222-
fake.exception = undefined;
223-
fake.exceptionCreator = undefined;
224-
fake.fakeFn = undefined;
225-
fake.callsThrough = false;
226228

227229
return fake;
228230
},
229231

230232
resolvesThis: function resolvesThis(fake) {
231-
fake.returnValue = undefined;
232-
fake.resolve = false;
233+
resetReturnBehavior(fake);
233234
fake.resolveThis = true;
234-
fake.reject = false;
235-
fake.returnValueDefined = false;
236-
fake.exception = undefined;
237-
fake.exceptionCreator = undefined;
238-
fake.fakeFn = undefined;
239-
fake.callsThrough = false;
240235
},
241236

242237
callThrough: function callThrough(fake) {

test/stub-test.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,32 @@ describe("stub", function () {
225225
refute(fakeFn.called);
226226
});
227227

228+
it("supersedes previous returnsArg", function () {
229+
const stub = createStub();
230+
stub.returnsArg(0);
231+
stub.returns(42);
232+
233+
assert.equals(stub(99), 42);
234+
});
235+
236+
it("supersedes previous returnsThis", function () {
237+
const stub = createStub();
238+
stub.returnsThis();
239+
stub.returns(42);
240+
241+
assert.equals(stub.call({ x: 1 }), 42);
242+
});
243+
244+
it("supersedes previous throwsArg", function () {
245+
const stub = createStub();
246+
stub.throwsArg(0);
247+
stub.returns(42);
248+
249+
refute.exception(function () {
250+
assert.equals(stub(new Error("should not throw")), 42);
251+
});
252+
});
253+
228254
it("supersedes previous callThrough", function () {
229255
const obj = {
230256
fn() {
@@ -799,6 +825,26 @@ describe("stub", function () {
799825
);
800826
});
801827

828+
it("supersedes previous returnsArg", function () {
829+
const stub = createStub();
830+
stub.returnsArg(0);
831+
stub.throwsArg(0);
832+
833+
assert.exception(function () {
834+
stub(new Error("expected"));
835+
});
836+
});
837+
838+
it("supersedes previous returnsThis", function () {
839+
const stub = createStub();
840+
stub.returnsThis();
841+
stub.throwsArg(0);
842+
843+
assert.exception(function () {
844+
stub(new Error("expected"));
845+
});
846+
});
847+
802848
it("should be reset by .resetBehavior", function () {
803849
const stub = createStub();
804850

@@ -909,6 +955,16 @@ describe("stub", function () {
909955

910956
assert.same(obj.fn(), obj);
911957
});
958+
959+
it("supersedes previous returnsArg", function () {
960+
const instance = {};
961+
instance.stub = createStub();
962+
instance.stub.returnsArg(0);
963+
instance.stub.returnsThis();
964+
965+
assert.same(instance.stub("ignored"), instance);
966+
});
967+
912968
});
913969

914970
describe(".throws", function () {

0 commit comments

Comments
 (0)