Added more power to Assert

[thislooksfun]

List of changes:

• assertions.go:
  • Added not functionality
  • Added IsOK method
  • Added IsSameType method
  • Cleaned up useless variable declarations
  • Added optional messages to all functions
• goblin.go:
  • Added ExpectFail method - this allows for testing of failing methods
  • Added catch method - this will catch all panics thrown in an It block, and fail cleanly
  • Added ExpectPanic method - this will prevent the catch method causing a fail if it catches a panic, but will fail if the catch method does not catch a panic
• tlf-custom_test.go: (feel free to change that name)
  • Added tests for new items

[thislooksfun]

I looked through the CI logs, since it failed, and the only thing that failed was checking a message that I changed the format of. Check yourself if you don't believe me, but I think the new format is easier to read, which is why I changed it.

[xetorthio]

Can you post a capture of the message output here please?

[marcosnils]

@thislooksfun. I don't follow your statement about tests failing due to format change. I clearly see in the travis logs that the Timeout test is panicking. I haven't had the time to investigate why but it doesn't seem a format issue...

[thislooksfun]

@xetorthio:

@marcosnils: The timeout test failed, but it intentionally failed. If you look, the goblin script failed, but the actual test there passed. The only test that failed was expecting the error message to read "true expected true to be falsey, false is not true" but I changed it to be "Expected true to be falsey, false is not true" Just do a ctrl+f for "FAIL:" on the CI log page.

[marcosnils]

@thislooksfun I see. Can you please fix assertions_test.go with the new message format and rename tlf-custom_test.go to something more descriptive like assertions_extended_test.go or something like that?

Which are use cases for ExpectFail()?. I've never used a testing framework where you expect the test to fail, it either fails or you make it fail by calling the Fail() function but you don't expect it to fail. I'm wondering which are the scenarios you're dealing with because looking at the tests you've provided, for example:

g.It("IsTrue", func() {
        g.ExpectFail()                                                                                                                                                         
        g.Assert(false).IsTrue()
})

This really confuses me. Maybe I'm missing something but I've never find the use case where I needed a test to fail and assert that that's correct..

@xetorthio what do you think about this?

[xetorthio]

Yeah... I would love to see an example of how ExpectFail is being used.

On 21:50, Fri, May 15, 2015 Marcos Nils notifications@github.com wrote:

@thislooksfun https://github.com/thislooksfun I see. Can you please fix assertions_test.go with the new message format and rename tlf-custom_test.go to something more descriptive like assertions_extended_test.go or something like that?

Which are use cases for ExpectFail()?. I've never used a testing framework where you expect the test to fail, it either fails or you make it fail by calling the Fail() function but you don't expect it to fail. I'm wondering which are the scenarios you're dealing with because looking at the tests you've provided, for example:

g.It("IsTrue", func() { g.ExpectFail() g.Assert(false).IsTrue() })

This really confuses me. Maybe I'm missing something but I've never find the use case where I needed a test to fail and assert that that's correct..

@xetorthio https://github.com/xetorthio what do you think about this?

— Reply to this email directly or view it on GitHub https://github.com/franela/goblin/pull/48#issuecomment-102554939.

[thislooksfun]

I was mainly using it to be able to test that the assertions were working properly. Failing when they get a wrong input is half the test, and it looks much more elegant to have it pass when it expects a failure rather than look like it failed, but actually passing, like the timeout test you mentioned earlier. So if someone else wanted to make a custom assert library on top of goblin (like this one started as), it is easier and nicer looking when they try to test it. As for fixing the tests, I can do that no problem.

EDIT: An actual example, as I was on my phone before. Let's use the timeout test. As you can see, here is the current output. However, using ExpectFail, we can make it look much nicer: Also, the code is cleaner and easier to read.

[thislooksfun]

Better example. While converting some existing tests over to using ExpectFail, I came across this. It turns out 3 of the tests there (lines 330, 331, and 333) are failing, because they are supposed to fail, but aren't. That is exactly the situation ExpectFail is useful for. You just check to see if ANY of the tests fail, not all the ones you need to fail. ExpectFail is used only on one It at a time.

Incidentally, I was able to fix all but "Should fail if done has been called multiple times", which works sometimes, but not others.

[thislooksfun]

Ok, in that new build the only failing test is the aforementioned "Should fail if done has been called multiple times" which as far as I can tell, is random in whether or not it succeeds.

Final update: As you can see here, I ran 1000 iterations of the test sequence. Out of that a total of 395 of the tests failed in some way. 100% (395) of the failed tests failed only in the "Should fail if done has been called multiple times" section. And of those, 153 panicked with the following message: "panic: runtime error: invalid memory address or nil pointer dereference"

[thislooksfun]

I hate to be that guy, but any update on this?

[marcosnils]

@thislooksfun promise I'll take a look tomorrow.

[artem-malko]

@marcosnils up) @thislooksfun looks like, we need rebase)

Come on, lets review this request!)

[thislooksfun]

@artem-malko I no longer use go, nor do I have time to check through this PR to see what would need to be changed. If someone wants to update this PR they can submit a PR to the master branch of thislooksfun/goblin, or open a new PR here.

[marcosnils]

@artem-malko if you're a Goblin user and wish to push this, happy to review since I'm looking at this repo these days... unfortunately I don't have time to rebase now :(

[artem-malko]

@marcosnils I'll do it on the closest weekend)