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

Using callThrough with a mock expectation throws an 'apply' of undefined error. #1442

Closed
jamespedid opened this issue May 31, 2017 · 6 comments

Comments

@jamespedid
Copy link

jamespedid commented May 31, 2017

  • Sinon version : 2.3.2
  • Environment : Node
  • Example URL : N/A
  • Other libraries you are using: Mocha, Chai, Chai as Promised

What did you expect to happen?
The mock expectation should support the full stub API, including callThrough. The mocked expectation should invoke the actual app code.

What actually happens
An error is thrown trying to use the callThrough method because this.stub.wrappedMethod appears to be undefined.

How to reproduce

describe('test', function () {
    it('should be ok', function () {
        const transaction = Object.create({
            commit: function () { return Promise.reject(); },
            rollback: function () { return Promise.resolve(); }
        });

        function appCode() {
            return transaction.commit();
        }

        const mock = sinon.mock(transaction);
        mock.expects('commit').once().callThrough();

        return expect(appCode()).to.eventually.be.fulfilled.then(() => mock.verify())
    });
});
Stack Trace
TypeError: Cannot read property 'apply' of undefined
  at Object.invoke (node_modules/sinon/lib/sinon/behavior.js:146:43)
  at Object.functionStub (node_modules/sinon/lib/sinon/stub.js:88:53)
  at Function.invoke (node_modules/sinon/lib/sinon/spy.js:194:51)
  at Function.invoke (node_modules/sinon/lib/sinon/mock-expectation.js:80:26)
  at Object.proxy (node_modules/sinon/lib/sinon/spy.js:97:22)
  at Object.invokeMethod (node_modules/sinon/lib/sinon/mock.js:132:43)
  at Object.commit (node_modules/sinon/lib/sinon/mock.js:66:35)
  at appCode (test/unit/graphql/models/Workflow.test.js:40:36)
@tonylukasavage
Copy link

tonylukasavage commented Jun 5, 2017

  • Sinon version : 2.3.2
  • Environment : Node

I've encountered the same error, as well as an additional one to go with it, so I distilled it down to as small an example as I could. The following example shows the error @jamespedid cites above, as well as shows that callThrough() is not behaving as expected. I'll leave it to the maintainer to decide whether these are related:

let sinon = require('sinon');

let obj = { query: function() { return 'foo'; } };

sinon.stub(obj, 'query')
	.withArgs(123)
	.returns('stubbed')
	.callThrough();

console.log(obj.query());     // prints "undefined"
console.log(obj.query(123));  // throws error 

stack trace

❯ node sinon-test.js
undefined
/Users/tony/development/innovu/app-server/node_modules/sinon/lib/sinon/spy.js:222
            throw exception;
            ^

TypeError: Cannot read property 'apply' of undefined
    at Object.invoke (node_modules/sinon/lib/sinon/behavior.js:146:43)
    at Object.functionStub (node_modules/sinon/lib/sinon/stub.js:88:53)
    at Function.invoke (node_modules/sinon/lib/sinon/spy.js:194:51)
    at Function.invoke (node_modules/sinon/lib/sinon/spy.js:192:40)
    at Object.proxy [as query] (node_modules/sinon/lib/sinon/spy.js:97:22)
    at Object.<anonymous> (sinon-test.js:11:17)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)

So both the withArgs() and callThrough() are having problems for me.

@tonylukasavage
Copy link

A little more poking around has shown this to be specific to withArgs() for me. In the absence of withArgs() the callThrough() function works as expected. It's as though the wrappedMethod is being lost when withArgs() is applied.

@tonylukasavage
Copy link

tonylukasavage commented Jun 5, 2017

OK, so I worked around the problem in a way that should give someone a little more knowledgeable in the sinon codebase something to go on. Everything on my end seems to work if callThrough() is called in the chain BEFORE withArgs(), or presumably any other fake generating code.

this ends in a crash:

let stub = sinon.stub(obj, 'query')
	.withArgs(123)
	.returns('bar')
	.callThrough();

while this behaves exactly as expected:

let stub = sinon.stub(obj, 'query')
	.callThrough()
	.withArgs(123)
	.returns('bar');

So it seems, IMHO, the callThrough() should be smart enough to traverse through the parent hierarchy to find the root of the stub and use its wrappedMethod(). I believe the problem section is here:

https://github.com/sinonjs/sinon/blob/master/lib/sinon/behavior.js#L146

This code should perhaps traverse up the stub parents to find the wrappedMethod when in a chaining scenario, which I believe would fix this bug. If this is a satisfactory approach I could put together a PR, but I'd feel better about getting a maintainers opinion on this approach to solving the problem.

In he meantime, call callThrough() before other fake functions in your chain and you should be good.

@tonylukasavage
Copy link

bump. Any thoughts from maintainers/contributors here?

@fatso83
Copy link
Contributor

fatso83 commented Jun 12, 2017

Thanks for taking the time to dig into this, Tony! Not intentionally meaning to leave you hanging. It's just that stuff like this is time consuming to debug, and when it doesn't affect oneself there are often other issues that are more compelling (ie more issues closed per day).

I don't have time to look into this right now, but if you manage to make a PR that fixes the issue then it's easier to reason around anyway.

@killmenot
Copy link
Contributor

I think when expectation is initiated and the method is wrapped, wrapMethod props do not extend expectation (https://github.com/sinonjs/sinon/blob/master/lib/sinon/mock.js#L66)

I believe that

var expectation = mockExpectation.create(method);

should be rewritten to

var expectation = mockExpectation.create(method);
extend(expectation, this.object[method]);

killmenot added a commit to killmenot/sinon that referenced this issue Sep 3, 2017
killmenot added a commit to killmenot/sinon that referenced this issue Sep 3, 2017
@killmenot killmenot mentioned this issue Sep 3, 2017
2 tasks
fatso83 added a commit that referenced this issue Sep 12, 2017
@mgabeler-lee-6rs mgabeler-lee-6rs mentioned this issue Apr 30, 2019
2 tasks
franck-romano pushed a commit to franck-romano/sinon that referenced this issue Oct 1, 2019
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

4 participants