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

Piping message through should syntax, better Throws verbosity #96

Merged
merged 5 commits into from Oct 1, 2012

Conversation

@scottnonnenberg
Copy link
Contributor

@scottnonnenberg scottnonnenberg commented Sep 5, 2012

Found myself having difficulties with 'expected 5 to equal 3' or 'expected true to equal false' messages, I thought I'd make it easier to give myself debugging hints later.

Also, I want to know what happened when an unexpected exception is thrown, so I made it give me more detail.

Also of note:

  • keys() does fancy stuff with its arguments, so I couldn't add a msg parameter. throws() similarly had a number of parameters, so you only get the message piped through if you give it both an exception and a message to look for within that exception.
  • added some missing root should tests for equal and Throw
  • added some tests to ensure that the inner length checks for above/below/within still pipe the message through
'cause we actually want to know what was thrown.
@travisbot
Copy link

@travisbot travisbot commented Sep 5, 2012

This pull request passes (merged 941f992 into e8cc23d).

(tests passing in Chrome 21.0.1180.89 and Firefox 15. Safari 6.0 (7536.25) passes all tests except for the three includeStack tests.
@travisbot
Copy link

@travisbot travisbot commented Sep 5, 2012

This pull request passes (merged 57a6e04 into e8cc23d).

@domenic
Copy link
Contributor

@domenic domenic commented Sep 5, 2012

Can you give some examples of what this would look like? (E.g. so we could include them in the docs.) Also, any reason not to add it to expect?

I just got approached at NodeConf summer camp about what I think is the same feature. He said "it's the one thing missing from Chai" that he got in should.js. So, +1

@logicalparadox

This comment has been minimized.

Copy link

@logicalparadox logicalparadox commented on lib/chai/core/assertions.js in e383a6e Sep 5, 2012

Style changes:

  • Since on one line, the { and } are unnecessary.
  • You can shorten the if statement to if (msg)...
@logicalparadox
Copy link
Member

@logicalparadox logicalparadox commented Sep 5, 2012

The other issue I see is that if your using the expect interface you can do the following...

expect(14, 'msg').to.be.a('number', 'anthrmsg');

...in which case the original msg would be overwritten. Something to note for the docs.

@scottnonnenberg
Copy link
Contributor Author

@scottnonnenberg scottnonnenberg commented Sep 5, 2012

It actually works for Expect, based on where I made the changes. Just added a commit including this new behavior in the Expect tests.

Example:

expect(temp).to.be.within(70, 80, 'comfortable temperature');
temp.should.be.within(70, 80, 'comfortable temperature');

On failure, the error message would be:

"comfortable temperature: expected 60 to be within 70..80"
I've been writing too much coffeescript!
@logicalparadox logicalparadox merged commit 00b2ed3 into chaijs:master Oct 1, 2012
1 check passed
1 check passed
default The Travis build passed
Details
@logicalparadox
Copy link
Member

@logicalparadox logicalparadox commented Oct 1, 2012

Thanks. I also added a bit of documentation to the inline comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants