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

Add test to assert exception message #283

Merged
merged 1 commit into from Jan 19, 2016
Merged

Add test to assert exception message #283

merged 1 commit into from Jan 19, 2016

Conversation

runlevel5
Copy link
Member

Related to #281

@jodosha
Copy link
Member

jodosha commented Jan 19, 2016

@joneslee85 In the original implementation of #281, @Moratorius used concrete examples of exceptions that Sequel can raise. They carry on useful messages for developers.

There is no value for them to see "Sequel::Error" without having a clue on what to do.

This PR should solve this problem. Can you please try to raise realistic exceptions and check if the returning message is the same? Thank you. 👍

See Acornsgrow@1db5273#diff-1ad76a79f04e29c81971a6206b63b136R85

@jodosha
Copy link
Member

jodosha commented Jan 19, 2016

@joneslee85 But we aren't testing against Sequel::CheckConstraintViolation and others exceptions. That was the point of this cleanup 😉

@jodosha
Copy link
Member

jodosha commented Jan 19, 2016

@joneslee85 Sweet, thank you very much 👍

runlevel5 pushed a commit that referenced this pull request Jan 19, 2016
Add test to assert exception message
@runlevel5 runlevel5 merged commit b7f6095 into hanami:master Jan 19, 2016
@runlevel5 runlevel5 deleted the enhance-test branch January 19, 2016 11:37
@jodosha jodosha added this to the v0.5.2 milestone Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants