franela / goblin

Minimal and Beautiful Go testing framework
MIT License
883 stars 79 forks source link

Added (optional) messages to Assertions #43

Closed theneubeck closed 9 years ago

theneubeck commented 9 years ago

I was missing the possibility to add a custom error message to an assertion. So I added it:

g.Assert("My string").Equal("Other string", "'Other' is not equal to 'My'")

And more useful:

g.Assert(user.IsValid()).IsTrue("Expected user to be valid")

plus

g.Assert(user.IsValid()).IsFalse("Expected user to be invalid")
marcosnils commented 9 years ago

@theneubeck thanks for contributing to goblin.

As a first basis I see that travis complained about tests not passing. Can you please review them and send the PR again?

As a second comment and per your examples I don't see how specifying a message could be useful in the Equal case. Because the equal assertion already prints the differences between the supplied arguments, so I your example I see that the message is somehow redundant. Can you please explain if you have some other cases in mind that specifying a message for Equal might be useful?

Regarding to adding a message to the IsTrue and IsFalse assertions I believe that it's a good idea as Goblin output is not very friendly when it comes to booleans.

theneubeck commented 9 years ago

Hi, sorry for the broken build (it doesn't seem to fail with my version of go) I'll review it.

Regarding the feature. It true that it was in the IsTrue/IsFalse case I felt the need to add a message. But since it seems pretty standard to be able to giva a custom message to assertEqual, I figured that I should add it, while at it. For example java's junit (http://junit.sourceforge.net/javadoc/org/junit/Assert.html) and python (https://docs.python.org/2/library/unittest.html) has that feature.

If you'd rather I remove the custom message from Equal and Eql, I can change it.

marcosnils commented 9 years ago

Let's stick with the message for the IsTrue and IsFalse for the moment. Whenever there's a real use case to add it to the Eql and Equal assertions we'll add it.

Please, go ahead and make the changes so we can review!.

marcosnils commented 9 years ago

I didn't see that you already made the change. Merging!.