Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

sinon.match.defined, sinon.assert.calledLike and friends #122

Merged
merged 6 commits into from

2 participants

@mantoni
Collaborator

Hello again,

I actually only wanted to quickly add sinon.match.defined and then start documenting, but then I couldn't resist writing an alternative to calledWith that uses sinon.match.like if the argument is a boolean, string, regexp or object. I'm sorry.

  • spy.firstCall.calledLike
  • spy.firstCall.notCalledLike
  • spy.calledLike
  • spy.alwaysCalledLike
  • spy.neverCalledLike
  • sinon.assert.calledLike
  • sinon.assert.alwaysCalledLike
  • sinon.assert.neverCalledLike
@cjohansen
Owner

Hehe, still on fire, eh? :) I think many of the examples are very interesting, but I am not convinced about this case:

spy(32);
spy.calledLike(true);

That seems sort of confusing, don't you think?

@mantoni
Collaborator

Yes. I Reviewed the buster documentation for assert.match again and suppose you would prefer the same behaviour here, which makes sense. That would mean that booleans are compared strictly.
On the other hand, sinon.match.like should be fuzzy here. The function name could be missleading then.
Do you think spy.calledWithMatch would be a better fit? It could implement behaviour for numbers and functions according to buster as well.

@cjohansen
Owner

I like that suggestion very much. In fact I've been meaning to write assert.calledWithMatch for buster for a while, having the functionality built into Sinon makes a lot more sense.

@mantoni
Collaborator

There are still two open points:

  1. There would be no difference between calledWith(true) and calledWithMatch(true). I would still vote for true(ish) matches in that case.
  2. The naming becomes inconsistent. The behaviour of calledWithMatch is then similar or even equal to sinon.match.like. Now that like does behave differently depending on the type of the given value, it could be moved into sinon.match as well. That would align the naming and the implementation would be even closer to Buster - except for the boolean case :-/
@cjohansen
Owner

So the only issue is that Sinon's matcher treats true as truthy and Buster treats it as boolean true?

@mantoni
Collaborator

Apparently :) Do you like the idea of moving 'like' into 'match'? I'm happy to change all of this and update the pull request if you do.

@cjohansen
Owner

I like that approach. Regarding true/truthy - I think that sinon.match.truthy is more explicit and less magic. What do you think?

@mantoni
Collaborator

Fair enough. Truthy sounds nice. I'll add that one as well then.

@mantoni
Collaborator

Oups! Synced the branch half way through. sinon.match needs to check for objects with a test function and numbers.

@cjohansen
Owner

So I shouldn't pull this yet, right?

@mantoni
Collaborator

Correct. Almost there. Maybe I can finish tonight.

@mantoni
Collaborator

Now you can pull :-)

@cjohansen
Owner

Sorry for the delay, I'm too far away from inbox 0...

@cjohansen cjohansen merged commit 98e9bfb into cjohansen:master
@mantoni
Collaborator

Buster seems to keep you quite busy. Can you update the sinon-next.js and sinon-ie-next.js files?

@cjohansen
Owner

Yeah, most of my open source time has been going into Buster lately. Sinon is still a priority though, it's just so much more complete than Buster, so need to bring Buster up to the same level :)

Updated the next files now. Also, I think I've figured out why the sinonjs.org server has been so fucking slow. Hopefully that problem is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.