Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

returns not override callThrough #2566

Closed
rluvaton opened this issue Oct 31, 2023 · 4 comments
Closed

returns not override callThrough #2566

rluvaton opened this issue Oct 31, 2023 · 4 comments

Comments

@rluvaton
Copy link
Contributor

rluvaton commented Oct 31, 2023

Describe the bug
returns not override callThrough

To Reproduce
Run the following:

const sinon = require('sinon');
const assert = require('assert');
class A {
	fn() {
		return 1;
	}
}

const a = new A();

sinon.stub(a, 'fn').callThrough().returns(2);

assert.strictEqual(a.fn(), 2);
assert.strictEqual(a.fn(), 2);
assert.strictEqual(a.fn(), 2);
assert.strictEqual(a.fn(), 2);

Expected behavior
this function should return 2 without calling the original function

as resolves and throws does (and maybe even more)

Context (please complete the following information):
The reason I call callThrough is because I have a helper function that stub the function but keeping the result (I know you have spy but stub can provide both override and tracking) or if already stubbed does nothing (so there won't be an error for function already stubbed)

replacing returns with callsFake works as expected

  • Library version: 17.0.0
  • Environment: node
@fatso83
Copy link
Contributor

fatso83 commented Oct 31, 2023

Thanks for the issue and the PR!

@fatso83 fatso83 modified the milestone: v18 Oct 31, 2023
@rluvaton
Copy link
Contributor Author

rluvaton commented Nov 1, 2023

Closed as done

@rluvaton rluvaton closed this as completed Nov 1, 2023
@fatso83
Copy link
Contributor

fatso83 commented Nov 1, 2023

we don't have LTS releases :) It's just the next major version (which I am not sure when lands).

Anyway, at first I regarded this as a breaking change, but it's really just fixing a bug, so the new patch version is out now.

@fatso83
Copy link
Contributor

fatso83 commented Apr 25, 2024

For later reference, the clearing of state turned out too be a bit too aggressive, so fixed this up in #2593

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

No branches or pull requests

2 participants