Add more descriptive message to missing key exception #38
Conversation
|
@jsl Already had this when I read your comment. But anyone else can take it from here. |
| @@ -10,7 +10,7 @@ | |||
| it "fails when the placeholder name is not in the given Hash" do | |||
| ->(){ | |||
| target.new('not_in_substitutions', 0, true).render(substitutions) | |||
| }.must_raise KeyError | |||
| }.must_raise RuntimeError | |||
There was a problem hiding this comment.
@mcelicalderon this looks good. sorry I didn't review this for so long. But, how about re-raising the KeyError with the more specific error, instead of the more generic RuntimeError? I think KeyError is really a good exception type for this error. Then we can just delete the change to the test and it should pass, right?
There was a problem hiding this comment.
Yes, totally makes sense, I'll change that.
|
@acamino can you please add a test to ensure that the error text is ok (you may want to tweak so it reads bette), and what we expect? Also, change to KeyError instead of RuntimeError. |
|
Actually, what do you think about making it a custom error like |
|
I'll go for adding a custom error. |
- Use `#` to make it explicit the test describes an instance method. - Put the example that corresponds to the happy path in the beginning. - Remove unnecessary parens from lambda expression. When there are no arguments it is safe to ommit the parens.
|
@jsl CI was complaining because this branch didn't have a .travis.yml. I've just pulled it from master and now we're green. |
|
LGTM! |
This should take care of #24, adds a descriptive error about a missing key in the hash passed to the render method on a
SimpleTemplates::Templateobject.