From 2b77b77d6a09981ddeaff4cf67e08904d42fbcd5 Mon Sep 17 00:00:00 2001 From: Vishal Kumar Singh Date: Sat, 16 May 2026 13:45:43 +0530 Subject: [PATCH 1/2] fix: isolate callId counter per sandbox for parallel test support The global callId counter in proxy-invoke.js caused calledImmediatelyBefore and calledImmediatelyAfter to fail when tests run in parallel with separate sandboxes. Each sandbox now maintains its own callId counter, passed through the spy/stub/fake creation chain via a context object. Fixes #2472 --- src/sinon/fake.js | 58 ++++++++++++++++++++++------- src/sinon/proxy-invoke.js | 10 +++-- src/sinon/proxy.js | 15 +++++++- src/sinon/sandbox.js | 31 +++++++++++++--- src/sinon/spy.js | 62 ++++++++++++++++++++++++++----- src/sinon/stub.js | 78 ++++++++++++++++++++++++++++++++------- test/src/sandbox-test.js | 65 ++++++++++++++++++++++++++++++++ 7 files changed, 272 insertions(+), 47 deletions(-) diff --git a/src/sinon/fake.js b/src/sinon/fake.js index 6dedf1528..bacc2320d 100644 --- a/src/sinon/fake.js +++ b/src/sinon/fake.js @@ -17,31 +17,57 @@ const { slice } = prototypes.array; * When an `f` argument is supplied, this implementation will be used. * * @param {SinonFunction|undefined} [f] + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} * @namespace */ -function fake(f) { - if (arguments.length > 0 && typeof f !== "function") { +function fakeImpl(f, context) { + if (typeof f !== "undefined" && typeof f !== "function") { throw new TypeError("Expected f argument to be a Function"); } - return wrapFunc(f); + return wrapFunc(f, context); } +/** + * Returns a `fake` that records all calls, arguments and return values. + * + * When an `f` argument is supplied, this implementation will be used. + * + * @param {SinonFunction|undefined} [f] + * @returns {SinonFunction} + * @namespace + */ +function fake(f) { + return fakeImpl(f, undefined); +} + +/** + * Creates a fake with a specific context (for sandbox use). + * + * @param {object} context The sinon context for callId tracking + * @param {SinonFunction|undefined} [f] + * @returns {SinonFunction} + */ +fake.withContext = function (context, f) { + return fakeImpl(f, context); +}; + /** * Creates a `fake` that returns the provided `value`, as well as recording all * calls, arguments and return values. * * @memberof fake * @param {unknown} value + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} */ -fake.returns = function returns(value) { +fake.returns = function returns(value, context) { function f() { return value; } - return wrapFunc(f); + return wrapFunc(f, context); }; /** @@ -49,14 +75,15 @@ fake.returns = function returns(value) { * * @memberof fake * @param {unknown|Error} value + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} */ -fake.throws = function throws(value) { +fake.throws = function throws(value, context) { function f() { throw getError(value); } - return wrapFunc(f); + return wrapFunc(f, context); }; /** @@ -64,14 +91,15 @@ fake.throws = function throws(value) { * * @memberof fake * @param {unknown} value + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} */ -fake.resolves = function resolves(value) { +fake.resolves = function resolves(value, context) { function f() { return Promise.resolve(value); } - return wrapFunc(f); + return wrapFunc(f, context); }; /** @@ -79,14 +107,15 @@ fake.resolves = function resolves(value) { * * @memberof fake * @param {unknown} value + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} */ -fake.rejects = function rejects(value) { +fake.rejects = function rejects(value, context) { function f() { return Promise.reject(getError(value)); } - return wrapFunc(f); + return wrapFunc(f, context); }; /** @@ -138,10 +167,11 @@ let uuid = 0; * Creates a proxy (sinon concept) from the passed function. * * @private - * @param {SinonFunction} f + * @param {SinonFunction} f + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction} */ -function wrapFunc(f) { +function wrapFunc(f, context) { const fakeInstance = function () { let firstArg, lastArg; @@ -160,7 +190,7 @@ function wrapFunc(f) { return f && f.apply(this, arguments); }; - const proxy = createProxy(fakeInstance, f || fakeInstance); + const proxy = createProxy(fakeInstance, f || fakeInstance, context); Object.defineProperty(proxy, "name", { value: "fake", diff --git a/src/sinon/proxy-invoke.js b/src/sinon/proxy-invoke.js index 8a7433ab9..132c21488 100644 --- a/src/sinon/proxy-invoke.js +++ b/src/sinon/proxy-invoke.js @@ -7,9 +7,11 @@ const { push, forEach, concat } = prototypes.array; const ErrorConstructor = Error.prototype.constructor; const { bind } = Function.prototype; -let callId = 0; const maxSafeInteger = Number.MAX_SAFE_INTEGER; +// Default context for backward compatibility when used outside a sandbox +const defaultContext = { callId: 0 }; + /** * @callback SinonFunction * @param {...unknown} args @@ -26,8 +28,10 @@ const maxSafeInteger = Number.MAX_SAFE_INTEGER; */ export default function invoke(func, thisValue, args) { const matchings = this.matchingFakes(args); - const currentCallId = callId; - callId = callId >= maxSafeInteger ? 0 : callId + 1; + // Use the proxy's context if available, otherwise fall back to the default + const ctx = this.sinonContext || defaultContext; + const currentCallId = ctx.callId; + ctx.callId = ctx.callId >= maxSafeInteger ? 0 : ctx.callId + 1; let exception, returnValue; proxyCallUtil.incrementCallCount(this); diff --git a/src/sinon/proxy.js b/src/sinon/proxy.js index c61876ec1..82b2fae19 100644 --- a/src/sinon/proxy.js +++ b/src/sinon/proxy.js @@ -244,6 +244,13 @@ delegateToCalls(proxyApi, "alwaysReturned", false, "returned"); delegateToCalls(proxyApi, "calledWithNew", true); delegateToCalls(proxyApi, "alwaysCalledWithNew", false, "calledWithNew"); +/** + * Wraps a function with a proxy that preserves arity. + * + * @param {SinonFunction} func The function to wrap + * @param {SinonFunction} originalFunc The original function (for arity) + * @returns {SinonFunction} The wrapped proxy function + */ function wrapFunction(func, originalFunc) { const arity = originalFunc.length; let p; @@ -376,9 +383,10 @@ function wrapFunction(func, originalFunc) { * * @param {SinonFunction} func The original function * @param {SinonFunction} originalFunc The original function (for arity and name) + * @param {object} [context] The sinon context for callId tracking (typically a sandbox) * @returns {SinonFunction} The proxy function */ -export default function createProxy(func, originalFunc) { +export default function createProxy(func, originalFunc, context) { const proxy = wrapFunction(func, originalFunc); // Inherit function properties: @@ -388,5 +396,10 @@ export default function createProxy(func, originalFunc) { extend.nonEnum(proxy, proxyApi); + // Store context for use in invoke (for parallel test support) + if (context) { + extend.nonEnum(proxy, { sinonContext: context }); + } + return proxy; } diff --git a/src/sinon/sandbox.js b/src/sinon/sandbox.js index 2ebc5e165..e515e5ca4 100644 --- a/src/sinon/sandbox.js +++ b/src/sinon/sandbox.js @@ -89,6 +89,9 @@ export default function Sandbox(opts = {}) { let loggedLeakWarning = false; sandbox.leakThreshold = DEFAULT_LEAK_THRESHOLD; + // Context object for this sandbox, used to isolate callId between parallel tests + const sandboxContext = { callId: 0 }; + function addToCollection(object) { if ( push(collection, object) > sandbox.leakThreshold && @@ -196,7 +199,12 @@ export default function Sandbox(opts = {}) { } sandbox.spy = function () { - const createdSpy = sinonSpy.apply(sinonSpy, arguments); + // Use withContext to pass sandbox context for isolated callId tracking + const args = arrayProto.concat( + [sandboxContext], + arrayProto.slice(arguments), + ); + const createdSpy = sinonSpy.withContext.apply(sinonSpy, args); const result = commonPostInitSetup( arguments, createdSpy, @@ -217,7 +225,12 @@ export default function Sandbox(opts = {}) { extend(sandbox.spy, sinonSpy); sandbox.stub = function () { - const createdStub = sinonStub.apply(sinonStub, arguments); + // Use withContext to pass sandbox context for isolated callId tracking + const args = arrayProto.concat( + [sandboxContext], + arrayProto.slice(arguments), + ); + const createdStub = sinonStub.withContext.apply(sinonStub, args); const result = commonPostInitSetup( arguments, createdStub, @@ -535,7 +548,12 @@ export default function Sandbox(opts = {}) { }; sandbox.fake = function fake() { - const createdFake = sinonFake.apply(sinonFake, arguments); + // Use withContext to pass sandbox context for isolated callId tracking + const args = arrayProto.concat( + [sandboxContext], + arrayProto.slice(arguments), + ); + const createdFake = sinonFake.withContext.apply(sinonFake, args); const result = commonPostInitSetup(arguments, createdFake, false); addToCollection(result); return result; @@ -559,10 +577,11 @@ export default function Sandbox(opts = {}) { }); function addFakeBehaviorToCollection(method) { - const original = sandbox.fake[method]; - sandbox.fake[method] = function () { - const result = original.apply(sinonFake, arguments); + // Add sandboxContext as the second argument for context-aware fakes + const args = arrayProto.slice(arguments); + args.push(sandboxContext); + const result = sinonFake[method].apply(sinonFake, args); addToCollection(result); return result; }; diff --git a/src/sinon/spy.js b/src/sinon/spy.js index 9c57ff503..f7edca92c 100644 --- a/src/sinon/spy.js +++ b/src/sinon/spy.js @@ -129,7 +129,14 @@ delegateToCalls( }, ); -function createSpy(func) { +/** + * Creates a spy from a function. + * + * @param {SinonFunction} func The function to spy on + * @param {object} [context] The sinon context for callId tracking + * @returns {SinonFunction} The spy + */ +function createSpy(func, context) { let name; let funk = func; @@ -141,14 +148,19 @@ function createSpy(func) { name = functionName(funk); } - const proxy = createProxy(funk, funk); + const proxy = createProxy(funk, funk, context); + + // Create a bound instantiateFake that preserves context + const instantiateFake = function (f) { + return createSpy(f, context); + }; // Inherit spy API: extend.nonEnum(proxy, spyApi); extend.nonEnum(proxy, { displayName: name || "spy", fakes: [], - instantiateFake: createSpy, + instantiateFake: instantiateFake, id: `spy#${uuid++}`, }); return proxy; @@ -160,39 +172,71 @@ function createSpy(func) { * @param {object|SinonFunction} [object] The object or function to spy on * @param {string} [property] The property name to spy on * @param {Array} [types] Types of accessor to spy on (get, set) + * @param {object} [context] The sinon context for callId tracking * @returns {SinonFunction|object} The spy or an object with spied accessors */ -export default function spy(object, property, types) { +function spyImpl(object, property, types, context) { if (isEsModule(object)) { throw new TypeError("ES Modules cannot be spied"); } if (!property && typeof object === "function") { - return createSpy(object); + return createSpy(object, context); } if (!property && typeof object === "object") { - return walkObject(spy, object); + return walkObject(function (obj, prop, propTypes) { + return spyImpl(obj, prop, propTypes, context); + }, object); } if (!object && !property) { return createSpy(function () { return; - }); + }, context); } if (!types) { - return wrapMethod(object, property, createSpy(object[property])); + return wrapMethod( + object, + property, + createSpy(object[property], context), + ); } const descriptor = {}; const methodDesc = getPropertyDescriptor(object, property); forEach(types, function (type) { - descriptor[type] = createSpy(methodDesc[type]); + descriptor[type] = createSpy(methodDesc[type], context); }); return wrapMethod(object, property, descriptor); } +/** + * Creates a spy (public API, backward compatible). + * + * @param {object|SinonFunction} [object] The object or function to spy on + * @param {string} [property] The property name to spy on + * @param {Array} [types] Types of accessor to spy on (get, set) + * @returns {SinonFunction|object} The spy or an object with spied accessors + */ +export default function spy(object, property, types) { + return spyImpl(object, property, types, undefined); +} + +/** + * Creates a spy with a specific context (for sandbox use). + * + * @param {object} context The sinon context for callId tracking + * @param {object|SinonFunction} [object] The object or function to spy on + * @param {string} [property] The property name to spy on + * @param {Array} [types] Types of accessor to spy on (get, set) + * @returns {SinonFunction|object} The spy or an object with spied accessors + */ +spy.withContext = function (context, object, property, types) { + return spyImpl(object, property, types, context); +}; + extend(spy, spyApi); diff --git a/src/sinon/stub.js b/src/sinon/stub.js index 4e84ed237..59d5d08fc 100644 --- a/src/sinon/stub.js +++ b/src/sinon/stub.js @@ -21,9 +21,22 @@ const pop = arrayProto.pop; const slice = arrayProto.slice; const sort = arrayProto.sort; +/** + * @callback SinonFunction + * @param {...unknown} args + * @returns {unknown} + */ + let uuid = 0; -function createStub(originalFunc) { +/** + * Creates a stub from a function. + * + * @param {SinonFunction} originalFunc The function to stub + * @param {object} [context] The sinon context for callId tracking + * @returns {SinonFunction} The stub + */ +function createStub(originalFunc, context) { // eslint-disable-next-line prefer-const let proxy; @@ -42,16 +55,21 @@ function createStub(originalFunc) { return getCurrentBehavior(fnStub).invoke(this, arguments); } - proxy = createProxy(functionStub, originalFunc || functionStub); + proxy = createProxy(functionStub, originalFunc || functionStub, context); // Inherit spy API: extend.nonEnum(proxy, spy); // Inherit stub API: extend.nonEnum(proxy, stub); + // Create a bound instantiateFake that preserves context + const instantiateFake = function (f) { + return createStub(f, context); + }; + const name = originalFunc ? functionName(originalFunc) : null; extend.nonEnum(proxy, { fakes: [], - instantiateFake: createStub, + instantiateFake: instantiateFake, displayName: name || "stub", defaultBehavior: null, behaviors: [], @@ -63,18 +81,20 @@ function createStub(originalFunc) { return proxy; } -export default function stub(object, property) { - if (arguments.length > 2) { - throw new TypeError( - "stub(obj, 'meth', fn) has been removed, see documentation", - ); - } - +/** + * Implementation of stub with optional context. + * + * @param {object|undefined} object The object to stub + * @param {string|undefined} property The property name to stub + * @param {object} [context] The sinon context for callId tracking + * @returns {SinonFunction} The stub + */ +function stubImpl(object, property, context) { if (isEsModule(object)) { throw new TypeError("ES Modules cannot be stubbed"); } - throwOnFalsyObject.apply(null, arguments); + throwOnFalsyObject(object, property); if (isNonExistentProperty(object, property)) { throw new TypeError( @@ -98,18 +118,20 @@ export default function stub(object, property) { typeof actualDescriptor.value !== "function"); if (isStubbingEntireObject) { - return walkObject(stub, object); + return walkObject(function (obj, prop) { + return stubImpl(obj, prop, context); + }, object); } if (isCreatingNewStub) { - return createStub(); + return createStub(undefined, context); } const func = typeof actualDescriptor.value === "function" ? actualDescriptor.value : null; - const s = createStub(func); + const s = createStub(func, context); extend.nonEnum(s, { rootObj: object, @@ -128,6 +150,34 @@ export default function stub(object, property) { return isStubbingNonFuncProperty ? s : wrapMethod(object, property, s); } +/** + * Creates a stub (public API, backward compatible). + * + * @param {object} [object] The object to stub + * @param {string} [property] The property name to stub + * @returns {SinonFunction} The stub + */ +export default function stub(object, property) { + if (arguments.length > 2) { + throw new TypeError( + "stub(obj, 'meth', fn) has been removed, see documentation", + ); + } + return stubImpl(object, property, undefined); +} + +/** + * Creates a stub with a specific context (for sandbox use). + * + * @param {object} context The sinon context for callId tracking + * @param {object} [object] The object to stub + * @param {string} [property] The property name to stub + * @returns {SinonFunction} The stub + */ +stub.withContext = function (context, object, property) { + return stubImpl(object, property, context); +}; + function assertValidPropertyDescriptor(descriptor, property) { if (!descriptor || !property) { return; diff --git a/test/src/sandbox-test.js b/test/src/sandbox-test.js index 499b1bead..eeb4f34fc 100644 --- a/test/src/sandbox-test.js +++ b/test/src/sandbox-test.js @@ -2242,4 +2242,69 @@ describe("Sandbox", function () { assert.equals(sandbox.fake.length, 1); }); }); + + describe("call order isolation between sandboxes (issue #2472)", function () { + it("calledImmediatelyBefore works correctly with separate sandboxes", function () { + const sandbox1 = createSandbox(); + const sandbox2 = createSandbox(); + + const spy1a = sandbox1.spy(); + const spy1b = sandbox1.spy(); + const spy2a = sandbox2.spy(); + const spy2b = sandbox2.spy(); + + spy1a(); + spy2a(); + spy1b(); + spy2b(); + + assert.isTrue(spy1a.calledImmediatelyBefore(spy1b)); + assert.isTrue(spy2a.calledImmediatelyBefore(spy2b)); + + sandbox1.restore(); + sandbox2.restore(); + }); + + it("calledImmediatelyAfter works correctly with separate sandboxes", function () { + const sandbox1 = createSandbox(); + const sandbox2 = createSandbox(); + + const spy1a = sandbox1.spy(); + const spy1b = sandbox1.spy(); + const spy2a = sandbox2.spy(); + const spy2b = sandbox2.spy(); + + spy1a(); + spy2a(); + spy1b(); + spy2b(); + + assert.isTrue(spy1b.calledImmediatelyAfter(spy1a)); + assert.isTrue(spy2b.calledImmediatelyAfter(spy2a)); + + sandbox1.restore(); + sandbox2.restore(); + }); + + it("stubs in different sandboxes have isolated callIds", function () { + const sandbox1 = createSandbox(); + const sandbox2 = createSandbox(); + + const stub1a = sandbox1.stub(); + const stub1b = sandbox1.stub(); + const stub2a = sandbox2.stub(); + const stub2b = sandbox2.stub(); + + stub1a(); + stub2a(); + stub1b(); + stub2b(); + + assert.isTrue(stub1a.calledImmediatelyBefore(stub1b)); + assert.isTrue(stub2a.calledImmediatelyBefore(stub2b)); + + sandbox1.restore(); + sandbox2.restore(); + }); + }); }); From d216f3f933e066c9236fa1317fad88c1f4c17aa0 Mon Sep 17 00:00:00 2001 From: Vishal Kumar Singh Date: Tue, 16 Jun 2026 20:22:33 +0530 Subject: [PATCH 2/2] fix: preserve fake undefined argument validation --- src/sinon/fake.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sinon/fake.js b/src/sinon/fake.js index bacc2320d..03a405d30 100644 --- a/src/sinon/fake.js +++ b/src/sinon/fake.js @@ -18,11 +18,12 @@ const { slice } = prototypes.array; * * @param {SinonFunction|undefined} [f] * @param {object} [context] The sinon context for callId tracking + * @param {boolean} hasFunctionArgument * @returns {SinonFunction} * @namespace */ -function fakeImpl(f, context) { - if (typeof f !== "undefined" && typeof f !== "function") { +function fakeImpl(f, context, hasFunctionArgument) { + if (hasFunctionArgument && typeof f !== "function") { throw new TypeError("Expected f argument to be a Function"); } @@ -39,7 +40,7 @@ function fakeImpl(f, context) { * @namespace */ function fake(f) { - return fakeImpl(f, undefined); + return fakeImpl(f, undefined, arguments.length > 0); } /** @@ -50,7 +51,7 @@ function fake(f) { * @returns {SinonFunction} */ fake.withContext = function (context, f) { - return fakeImpl(f, context); + return fakeImpl(f, context, arguments.length > 1); }; /**