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

sinon.stub().resolves(...) returns undefined when used with createStubInstance #2073

Closed
clemmy opened this issue Aug 7, 2019 · 7 comments
Closed

Comments

@clemmy
Copy link

clemmy commented Aug 7, 2019

Describe the bug
This was working in 7.3.2, but broke in 7.4.1.
Basically, when calling sinon.stub().resolves(...) as part of the second argument of sinon.createStubInstance(), I get undefined. It works as a standalone, however.

To Reproduce
Steps to reproduce the behavior:
Run the following test:

class X {
  async test() {
    return Promise.resolve(1)
  }
}

it('is a buggy test', async () => {
  const thing = sinon.createStubInstance(X, {
    test: sinon.stub().resolves(2) as sinon.SinonStub<[], Promise<number>>
  })
  console.log(thing.test()) // undefined
}

Expected behavior
I would expect the console.log(thing.test()) to output 2

Screenshots
N/A

Context (please complete the following information):

  • Library version: 7.4.1
  • Environment: Mac OSX
  • Example URL: n/a
  • Other libraries you are using: n/a

Additional context
N/A

@fatso83
Copy link
Contributor

fatso83 commented Aug 8, 2019

Not sure what could cause this in the changelog ... If you could run a git bisect run to find out which commit introduced this you can use this template: https://github.com/fatso83/git-bisect-scripts/blob/master/full-template.sh

@rgroothuijsen
Copy link
Contributor

git bisect reveals that createStubInstance was removed from the sinon API in this commit.

@joebowbeer
Copy link

I can verify that it was broken in 7.4.0 as well

@kevinbosman
Copy link
Contributor

Also related to this, due to createStubInstance now using the (different) Sandbox implementation, it can no longer be called without explicitly specifying the Sandbox object binding.

So the following Typescript import no longer works (it worked in 7.3.2):

import { createStubInstance } from "sinon";
...
const thing = createStubInstance(X);

which results in TypeError: Cannot read property 'stub' of undefined

This could be resolved by replacing this.stub with sandbox.stub here:

return this.stub(Object.create(constructor.prototype));

kevinbosman added a commit to kevinbosman/sinon that referenced this issue Aug 14, 2019
PR sinonjs#2022 redirected sinon.createStubInstance() to use the Sandbox
implementation thereof. This introduced a breaking change due to the
sandbox implementation not supporting property overrides.

Instead of duplicating the original behaviour from stub.js into
sandbox.js, call through to the stub.js implementation then add all
the stubs to the sandbox collection as usual.

Copy the missing tests from stub-test.js
kevinbosman added a commit to kevinbosman/sinon that referenced this issue Aug 14, 2019
PR sinonjs#2022 redirected sinon.createStubInstance() to use the Sandbox
implementation thereof. This introduced a breaking change due to the
sandbox implementation not supporting property overrides.

Instead of duplicating the original behaviour from stub.js into
sandbox.js, call through to the stub.js implementation then add all
the stubs to the sandbox collection as usual.

Copy the missing tests from stub-test.js and add issue tests.
@fatso83
Copy link
Contributor

fatso83 commented Aug 15, 2019

This issue, and some others, has revealed to me that, while we have great unit test coverage, some integration tests for the API is missing. We should have a little test suite that just tests for and executes a base test for all of the exposed API methods. Then stuff like this wouldn't happen nilly-willy.

@fatso83
Copy link
Contributor

fatso83 commented Aug 15, 2019

PS. Great work on digging into this guys!

mroderick pushed a commit that referenced this issue Sep 2, 2019
PR #2022 redirected sinon.createStubInstance() to use the Sandbox
implementation thereof. This introduced a breaking change due to the
sandbox implementation not supporting property overrides.

Instead of duplicating the original behaviour from stub.js into
sandbox.js, call through to the stub.js implementation then add all
the stubs to the sandbox collection as usual.

Copy the missing tests from stub-test.js and add issue tests.
@mroderick
Copy link
Member

This has been fixed with #2073 and released as sinon@7.4.2 🚀

franck-romano pushed a commit to franck-romano/sinon that referenced this issue Oct 1, 2019
PR sinonjs#2022 redirected sinon.createStubInstance() to use the Sandbox
implementation thereof. This introduced a breaking change due to the
sandbox implementation not supporting property overrides.

Instead of duplicating the original behaviour from stub.js into
sandbox.js, call through to the stub.js implementation then add all
the stubs to the sandbox collection as usual.

Copy the missing tests from stub-test.js and add issue tests.
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

6 participants