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

Verboser error message #1

Merged
merged 1 commit into from
Oct 25, 2015

Conversation

mikegee
Copy link
Contributor

@mikegee mikegee commented Oct 21, 2015

Spell out the problem in the error message for user-friendliness.

Motivation: When this error is raised while Rails is rendering a view, Rails helpfully rescues it and re-raises it as an ActionView::Template::Error. So, I was getting just ActionView::Template::Error: 0, which is less than helpful.

Also, rename LengthNotOne to CountNotOne since we use the count method and not the length method.

Spell out the problem in the error message for user-friendliness.

Motivation: When this error is raised while Rails is rendering a view,
Rails helpfully rescues it and re-raises it as an
`ActionView::Template::Error`. So, I was getting just
`ActionView::Template::Error: 0`, which is less than helpful.

Also, rename `LengthNotOne` to `CountNotOne` since we use the `count`
method and not the `length` method.
raise_error(Enumerable::FirstAndOnly::LengthNotOne, "0")
expect { subject.first_and_only! }.to raise_error(
Enumerable::FirstAndOnly::CountNotOne,
"Expected the count to be 1, but it was 0."
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm doing it wrong, but the original intent was that the error class itself would be part of the error message:

Enumerable::FirstAndOnly::CountNotOne # "Oh, I see the count is supposed to be 1"

So putting that in the text of the message is kind of redundant:

Expected the count to be 1, but it was 0.

So, I should mark this as "wont-fix, change the downstream problem with ActionView" :trollface:

So thinking about this in real time, I think I agree with your change, even though it duplicates information in the class name. For example, maybe the text of the error message could be localized, but of course, the CountNotOne class name would not be localized.

Thanks @mikegee!

markjlorenz added a commit that referenced this pull request Oct 25, 2015
@markjlorenz markjlorenz merged commit 77fa3e2 into markjlorenz:master Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants