fix assertEquals() if toString() returns null

[panchenko]

Fixes #1453

[marcphilipp]

Why do we need this fail statement?

[panchenko]

Only in case assertEquals() does nothing.

[marcphilipp]

Well, the same holds true for all the other tests in this class, right? If so, please remove the return and the fail() statements.

[panchenko]

@marcphilipp there are different kind of tests in this class:

• some have @Test(expected = AssertionError.class)
• some have return + fail()
• some have only fail() inside try-catch
• some don't have fail() / return

As this test is expected to throw an exception and that exception is being checked, I think it's better to make sure an exception was thrown. I agree this variant is too verbose, the best approach would be throwing some new runtime exception from the try-catch block.

[kcooney]

Calling fail from inside of a try block that has catch (AssertionError doesn't do what many people think it does, so I'm strongly against that option. Maybe I'll find time to fix the tests that do that.

I also dislike using @Test(expected = Something.class) for any test method where there is more than one call in the body, since it isn't clear which method should throw the exception. Because using expected should usually only be used for single-method tests, I tend to completely avoid it in my personal projects, but it's usage is very common in the JUnit tests.

Throwing a custom exception inside the try block for something that should be an assertion error is a bit odd, so I'm not thrilled with that option.

I personally strongly in favor of the return and fail idiom for any tests that verify that a test method throws AssertionError unless the test can be written with one line of code (in which case @Test(expected = AssertionError.class) is fine).

Luckily, with Java 8 we can all use lambdas so this will all be moot some day.

[panchenko]

I would agree with "a bit odd", however it's shorter. Will revert back to return :-)

[panchenko]

@kcooney I would like this extracted as a function, and will update other tests then separately.

[kcooney]

If we could could do that without hurting readability that is fine. For tests I am fine with some duplication if it makes the code clear.