Skip to content

Fix stub instance call order in sandboxes - #2761

Open
tianrking wants to merge 1 commit into
sinonjs:mainfrom
tianrking:fix/stub-instance-call-order
Open

tianrking wants to merge 1 commit into
sinonjs:mainfrom
tianrking:fix/stub-instance-call-order

Conversation

@tianrking

Copy link
Copy Markdown

Purpose (TL;DR) - mandatory

Fix #2760 so methods created by createStubInstance participate in call-order comparisons with spies and stubs from the same sandbox.

Background (Problem in detail)

Sandbox spies and stubs use a private call-ID context, but createStubInstance created its method stubs through the default global context. As a result, calledBefore and calledAfter compared unrelated counters and could report the wrong order.

Solution

Thread the sandbox call-ID context into stub-instance creation, while preserving the standalone helper's existing default behavior. The root Sinon API now delegates to its root sandbox implementation so collection and call ordering follow the same path.

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. npx mocha test/src/create-sinon-api-test.js test/src/sandbox-test.js --timeout 10000
  4. npm run lint

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.

@lbesecker195 lbesecker195 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the #2760 reproduction against src/sinon.js on main and on this branch. On main, sinon.createStubInstance(Foo).bar.calledAfter(spy) is false even though the stub is called second, and inside a sandbox spy.calledAfter(inst.bar) and inst.bar.calledImmediatelyBefore(spy) are false for the same reason (the instance methods get callIds from the module-level default context). With this branch all of those are true. A stub instance in one sandbox and a spy in another still start from independent counters (both [0]), so the per-sandbox isolation from #2715 is kept, and overrides plus restore on stubbed methods still work. After npm run build-artifacts, the node suite is 1558 passing / 12 pending versus 1556 / 12 on main, and eslint passes on the changed files. LGTM.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

createStubInstance() doesn't receive per-sandbox callId context, breaking calledAfter/calledBefore vs spy()

2 participants