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

core: improve message for assertion errors (throw assertion) #201

Merged
merged 2 commits into from Oct 23, 2013

Conversation

@andreineculau
Copy link
Contributor

@andreineculau andreineculau commented Oct 23, 2013

# create CustomError with prototype Error
fun = () -> throw new CustomError('test', true)
fun.should.not.Throw()

the above ends up showing an error message which is not useful at all (due to the Error instance being passed all the way to objDisplay) e.g.

AssertionError: expected [Function] to not throw an error but { Object (expected, found, ...) } was thrown

This PR, changes the message to e.g.

AssertionError: expected [Function] to not throw an error but 'CustomError: Expected x but found y' was thrown

@logicalparadox
Copy link
Member

@logicalparadox logicalparadox commented Oct 23, 2013

I like the change but the travis build failed. Can you please check the logs and refactor any tests so this passes for all environments.

@coveralls
Copy link

@coveralls coveralls commented Oct 23, 2013

Coverage Status

Coverage remained the same when pulling e174212 on andreineculau:patch-1 into 4b51ea7 on chaijs:master.

@andreineculau
Copy link
Contributor Author

@andreineculau andreineculau commented Oct 23, 2013

Sorry, I wasn't thinking straight.
I updated all relevant code, and the necessary tests.

@domenic
Copy link
Contributor

@domenic domenic commented Oct 23, 2013

From what I can tell of the tests, this change just replaces square brackets with single quotation marks. Why is that a good thing?

logicalparadox added a commit that referenced this pull request Oct 23, 2013
core: improve message for assertion errors (throw assertion)
@logicalparadox logicalparadox merged commit 1a4d35d into chaijs:master Oct 23, 2013
1 check passed
1 check passed
default The Travis CI build passed
Details
@logicalparadox
Copy link
Member

@logicalparadox logicalparadox commented Oct 23, 2013

Thanks! Worked great.

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
You can’t perform that action at this time.