Skip to content

fix: ensure last behavior setter always wins#2682

Open
ordinary9843 wants to merge 1 commit intosinonjs:mainfrom
ordinary9843:main
Open

fix: ensure last behavior setter always wins#2682
ordinary9843 wants to merge 1 commit intosinonjs:mainfrom
ordinary9843:main

Conversation

@ordinary9843
Copy link
Copy Markdown

Problem

When chaining stub behavior setters, the last call should always win. However, several combinations silently failed because flags set by earlier setters weren't cleared.

For example:

const stub = sinon.stub();
stub.returnsArg(0);
stub.returns(42); // should win, but doesn't — returnsArg silently persists
stub('ignored'); // returns 'ignored' instead of 42

This happens because behavior.invoke() checks returnArgAt (priority 2) before returnValue (priority 12). When returnsArg is called first, it sets returnArgAt. When returns is called next, it sets returnValue but leaves returnArgAt intact — so returnArgAt still takes effect.

The same issue affects returnsThis, throwsArg, and any setter that doesn't clear competing state.

Fix

Add a resetReturnBehavior() helper that clears all mutually exclusive return-behavior flags. Call it at the start of each setter so the most recently called setter always takes effect, regardless of the priority order in invoke().

Broken combinations fixed

Earlier setter Later setter Before After
returnsArg returns broken fixed
returnsThis returns broken fixed
throwsArg returns broken fixed
returnsArg throwsArg broken fixed
returnsThis throwsArg broken fixed
returnsArg returnsThis broken fixed

Fixes #2656

@fatso83
Copy link
Copy Markdown
Contributor

fatso83 commented Apr 9, 2026

Thanks for this, could you rebase on the latest main and I'll have a new look?

@fatso83
Copy link
Copy Markdown
Contributor

fatso83 commented Apr 9, 2026

I think some of these changes will be made redundant by #2670?

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.
@ordinary9843
Copy link
Copy Markdown
Author

Rebased on latest main. The test suite in test/src/ passes cleanly — the failures in test/issues/ are due to the ESM migration in #2683 (the issues test still uses require against the built lib/).

On the overlap with #2670: the two address different call orderings. #2670 cleans up setter state when callThrough() is the last call; this PR clears all competing flags when any setter is the last call (e.g. returnsArg then returns). Both are needed for consistent last-setter-wins behavior.

Also worth noting — after #2683, lib/ is a build artifact. #2670 currently targets lib/sinon/default-behaviors.js directly, so it will need updating before it can land.

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.

returns() doesn't override returnsArg()

2 participants