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

Merged
merged 6 commits into from Apr 30, 2012

Projects

None yet

2 participants

@mantoni
Member
mantoni commented Apr 17, 2012

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
Contributor

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
Member
mantoni commented Apr 20, 2012

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
Contributor

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
Member
mantoni commented Apr 21, 2012

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
Contributor

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

@mantoni
Member
mantoni commented Apr 22, 2012

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
Contributor

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

@mantoni
Member
mantoni commented Apr 23, 2012

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

@mantoni
Member
mantoni commented Apr 23, 2012

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

@cjohansen
Contributor

So I shouldn't pull this yet, right?

@mantoni
Member
mantoni commented Apr 24, 2012

Correct. Almost there. Maybe I can finish tonight.

@mantoni
Member
mantoni commented Apr 26, 2012

Now you can pull :-)

@cjohansen
Contributor

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

@cjohansen cjohansen merged commit 98e9bfb into sinonjs:master Apr 30, 2012
@mantoni
Member
mantoni commented Apr 30, 2012

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

@cjohansen
Contributor

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