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

[feature-request] Clean way to test a specific spy call #69

Open
jurko-gospodnetic opened this issue Sep 14, 2015 · 8 comments
Open

[feature-request] Clean way to test a specific spy call #69

jurko-gospodnetic opened this issue Sep 14, 2015 · 8 comments

Comments

@jurko-gospodnetic
Copy link

We can use a construct like:

expect(mySpy.secondCall).to.have.been.calledWithExactly(...);

and that works, but when that test fails, there is no mention of the fact that it was related to the second call in the test output. In fact, it displays the expected parameters and all the call information, which can get pretty confusing assuming that the expected call was made, just not as the second one.

It would be nice if sinon-chai gave us a way to cleanly test a specific spy call and report such failures in a way that would not require opening the test code to see what it was that was actually being expected.

One idea might be to support something like this:

expect(mySpy).to.have.call(2).calledWithExactly(...);

but possibly a solution can be found that would be more like a readable English sentence.

Many thanks!

@domenic
Copy link
Collaborator

domenic commented Sep 14, 2015

The correct approach here is to just create a separate test per call, or to use assertion message overrides.

Anyway, Sinon–Chai simple provides what Sinon provides, and since Sinon does not provide this sort of thing, I don't plan to add it to Sinon–Chai.

@domenic domenic closed this as completed Sep 14, 2015
@jurko-gospodnetic
Copy link
Author

Actually, sinon does provide this sort of thing, and that is exactly how I ran into it. :-)

People on my project were doing things like:

expect(spy.firstCall.calledWithExactly(...)).to.be.true;
expect(spy.secondCall.calledWithExactly(...)).to.be.true;
expect(spy.thirdCall.calledWithExactly(...)).to.be.true;

which did not give nice test failure messages (something like 'expected false to be true').

Doing this instead:

expect(spy.firstCall).to.have.been.calledWithExactly(...);
expect(spy.secondCall).to.have.been.calledWithExactly(...);
expect(spy.thirdCall).to.have.been.calledWithExactly(...);

is nicer but could be improved upon.

And btw. .firstCall, .secondCall, .thirdCall are just convenience shortcuts provided by sinon for a more generic .call(n) call.

@domenic
Copy link
Collaborator

domenic commented Sep 14, 2015

Oh! Well, those still work with Sinon–Chai chai.

expect(spy.firstCall).to.be.calledWithExactly(...)

@jurko-gospodnetic
Copy link
Author

Yeah, they work, but their test output is less than optimal. 😄

As described in my original issue description, when they fail, they output data passed in to all the calls, and there is no mention anywhere of what call the failure is being reported for.

For example this is based on an actual failure like that in one of our test suites (specific data & file names have been replaced so I don't have to go through the trouble of discovering what may or may not be publicly shareable):

  1) Funky little foo when bar-ed should get called with correct parameters:
     AssertionError: expected foo to have been called with exact arguments one, { two: 2 }, 5, 1, typeOf("function")
    foo(one, { two: 2 }, 5, 1, function f() {}) => true
    foo(x, { two: 2 }, 11, 1, function f() {}) => true
    foo(x, { two: 2 }, 2, 1, function f() {}) => true
      at my-tests/my-funky.test.spec.js:337:69

As you can see, the test failure output complains about foo not being called with some parameters when in fact the first call made to foo passed exactly those parameters.

The problem is that the actual test code was specifically testing the second call, and not any of them, and that is important information that would be very helpful if displayed in the test failure output.

The perfect test failure output for this case would be something like:

  1) Funky little foo when bar-ed should get called with correct parameters:
     AssertionError: expected foo called the second time with exact arguments one, { two: 2 }, 5, 1, typeOf("function")
    foo(x, { two: 2 }, 11, 1, function f() {}) => true
      at my-tests/my-funky.test.spec.js:337:69

@domenic
Copy link
Collaborator

domenic commented Sep 14, 2015

Hmm. And when you use Sinon directly, its assertion messages specify which call?

@domenic domenic reopened this Sep 14, 2015
@jurko-gospodnetic
Copy link
Author

Yup, just tried it using Sinon directly, and it does not specify the exact call index, but it does list only the one call being tested, so in the example used before it would output:

  1) Funky little foo when bar-ed should get called with correct parameters:
     AssertError: expected foo(x, { two: 2 }, 11, 1, function f() {}) => true to be called with exact arguments one, { two: 2 }, 2, 1, typeOf("function")
      at my-tests/my-funky.test.spec.js:337:69

which is good, as the reader now knows the exact call parameters being compared.

@JasonCust
Copy link

I just ran into a similar test scenario where the call is internally recursive so being able to assert/expect on separate calls would have been handy.

Is this on the roadmap?

@domenic
Copy link
Collaborator

domenic commented May 11, 2016

There is no "roadmap", but this project does accept pull requests.

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

3 participants