thoughtbot / cocaine

A small library for doing (command) lines.
https://robots.thoughtbot.com
Other
785 stars 55 forks source link

Use '==' for assertions against true and false #70

Closed jneander closed 10 years ago

jneander commented 10 years ago

This fixes tests breaking with non-monkey-patched 'true' and 'false' objects.

jyurek commented 10 years ago

I'm not sure what you mean by 'non-monkey patched'. Cocaine doesn't monkey patch true and the tests run fine. Why is this change necessary?

jneander commented 10 years ago

The previous implementation depends on true being patched with .true? as a valid method for assertions. In the current implementation of cocaine, given the current dependencies, .true? does not exist. The Travis CI builds are all failing for this reason, as were the tests when run locally after a clean clone. Changing the assertion corrects the problem.

jyurek commented 10 years ago

Ah! I see. I didn't realize this was the case. Thanks for letting me know! One thing, though. Our style guide (https://github.com/thoughtbot/guides/tree/master/style#testing) prefers eq to ==. So if you could change that, I'll pull this in right away. Thanks!

jneander commented 10 years ago

Thank you for mentioning the style guide. Consistency is valuable. I have converted those over and confirmed the tests locally. Pending the green light from Travis CI, this should be good to merge.

jyurek commented 10 years ago

Thanks very much for this!

jneander commented 10 years ago

You are most welcome!