Assertj-core: Assertions.assertThatThrownBy doesn't support the as/describedAs API

[kristiany]

This:

    Assertions.assertThatThrownBy(something::call)
        .as("This is not OK")
        .isInstanceOf(expectedException);

yields the default message:

    java.lang.AssertionError: Expecting code to raise a throwable.

[joel-costigliola]

Indeed, thanks for reporting this.

[kristiany]

Indeed, thanks for reporting this.

:+1:

[kristiany]

The thing is something::call gets evaluated in assertThatThrownBy (of course), so as never gets executed if call doesn't throw an exception. It's also not possible to set a description as the first call, because as and describedAs are not in the Assertions API, and perhaps that makes sense.

What if the Java 8 function is a regular assertThat construct/type, and evaluated with a thrownBy-method?

[joel-costigliola]

You are right, you can't pass a message to assertThatThrownBy, what you suggest is already possible by using catchThrowable inside assertThat:

assertThat(catchThrowable(() -> { throw new Exception("boom!") })).as("boom expected")
                  .isInstanceOf(Exception.class)
                  .hasMessageContaining("boom");

removing the throw will make the assertion fail:

java.lang.AssertionError: [boom expected] 
Expecting actual not to be null

[joel-costigliola]

It's worth documenting this assertThatThrownBy limitation and the given alternative in the javadoc.

[kristiany]

what you suggest is already possible by using catchThrowable inside assertThat

Nice, that works.

It's worth documenting this assertThatThrownBy limitation and the given alternative in the javadoc.

:+1:

[joel-costigliola]

Documented.

[mbjelac]

This results in code which begs for a comment. :(

assertThat(catchThrowable(() -> callSut())) // don't reduce to assertThatThrownBy because .as() won't work!
                .as("explanation")
                .isInstanceOf(ExpectedException.class);

Can this be fixed with assertThatThrownBy after all?

[joel-costigliola]

As mentioned by kristiany there is no obvious fix for that (I'm happy to be proven wrong though).
Which clarification do you want to see in assertThatThrownBy javadoc ?

[mbjelac]

No chance to override as() in AbstractThrowableAssert?

[joel-costigliola]

I just recall that I believe you can use assertThatCode in your case:

@Test
public void can_use_as_when_checking_exception() {

    final ThrowingCallable doNothing = () -> { };

     // assertion fails and "display me" appears in the assertion error
     assertThatCode(doNothing).as("display me")
                      .isInstanceOf(Exception.class);
}

Would that work for you ?
assertThatCode javadoc is a bit confusing (some copy paste from assertThatThrownBy) so I'll update it.

[mbjelac]

It works. But I am not thrilled. :)

• confusing method name
• it works the same as asserting on a caught throwable, e.g. nothing thrown results in Expecting actual not to be null, which is not as helpful as Expecting code to raise a throwable.

[joel-costigliola]

I do agree assertThatCode(doNothing).isInstanceOf(Exception.class); is a bit confusing, the main usage for it was assertThatCode(doNothing).doesNotThrowAnyExceptions().

I don't see any good options for this problem since as can not be supported in assertThatThrownBy but I might be missing something.

[mbjelac]

Yes, I (finally!) see the problem. assertThatThrownBy asserts not null on the throwable before any chained calls, as or otherwise. I suppose this was made so users can check whether the call through anything at all... I admit that I have indeed used this a couple of times.

The only solution I see is that we use assertThatCode.
However, the message in case of throwable being null should be fixed.

The fix: All public methods in AbstractThrowableAssert which check parts of the throwable (isSameAs, isInstanceOf, hasMessage, ...) should first call hasBeenThrown().

There is no point in checking a throwable which is null because this is de-facto usage of a part of the AbstractObjectAssert API not intended for throwables. A null throwable is already an error, not a regular value which should be checked.

And I would prefer the method assertThatCode be called assertThatCall since that's more specific and after all, its argument is called ...Callable.

[joel-costigliola]

I can agree with calling hasBeenThrown() in AbstractThrowableAssert.

We can't rename assertThatCode as it would be a breaking change, I'm a bit reluctant to add assertThatCall as an alias assertThatCode because users might wonder what is the difference with assertThatCode.

[mbjelac]

Great, I am pleased. :)

I'll probably get used to the name anyway.

[joel-costigliola]

Would you be keen to contribute that ?

[mbjelac]

Sure, I'll drop you a PR one of these days.

[mbjelac]

Riiight ... one of these days. Sorry. Got swamped with other stuff as usual.
In the meantime I stopped seeing this as important.
I'll fix it when/if I run into the issue again and it still bothers me.

[joel-costigliola]

Sure, no problem.