Assertj-core: Allow assertion on the root cause message

[wimdeblauwe]

Summary

It is currently not directly possible to assert on the root cause message. There is a workaround possible by using hasStackTraceContaining, but it would be nice if this was supported directly.

See also https://stackoverflow.com/questions/38943346/assertj-assert-on-the-cause-message

Example

I would like to be able to do this:

assertThatExceptionOfType(PersistenceException.class)
    .isThrownBy(() -> {
        // test code here
    })
   .withRootCauseInstanceOf(PSQLException.class)
   .withRootCauseMessageMatching("Key (user_id)=(.*) is not present in table \"book\"");

So to be consistent, probably the following new methods are needed:

• withRootCauseMessageStartingWith(String description)
• withRootCauseMessageContaining(String description)
• withRootCauseMessageContainingAll(CharSequence... values)
• withRootCauseMessageNotContaining(String content)
• withRootCauseMessageNotContainingAny(CharSequence... values)
• withRootCauseMessageMatching(String regex)
• withRootCauseMessageEndingWith(String description)

[scordio]

I just posted another answer in the SO question using AssertJ >= 3.14.

However, maybe a shortcut could be provided to change the assertion subject, e.g., getCause()/getRootCause() which would return an updated ThrowableAssert.

Would this help your use case?

[wimdeblauwe]

Interesting, but how can I combine that with assertThatExceptionOfType() ?

[scordio]

Interesting, but how can I combine that with assertThatExceptionOfType() ?

I just realized I was referring to the catchThrowable style, but your example mentioned assertThatExceptionOfType.

Unfortunately extracting is not available in the latter so we would need to extend ThrowableAssertAlternative.

[wimdeblauwe]

Consider this a request for adding extracting in that case :)

[scordio]

I see potentially two options here:

1. Let ThrowableAssertAlternative extends from AbstractObjectAssert similarly to AbstractThrowableAssert, this way extracting would be a quick win but we have a missing InstanceOfAssertFactory

assertThatExceptionOfType(PersistenceException.class)
    .isThrownBy(() -> {
        // test code here
    })
   .extracting(Throwable::getCause, as(THROWABLE_ALTERNATIVE)) // missing factory which would not follow the standard pattern
   .withMessageMatching("Key (user_id)=(.*) is not present in table \"book\"");

1. Add something like havingCause()/havingRootCause() to ThrowableAssertAlternative to change the assertion object

assertThatExceptionOfType(PersistenceException.class)
    .isThrownBy(() -> {
        // test code here
    })
   .havingCause() // any better name here?
   .withMessageMatching("Key (user_id)=(.*) is not present in table \"book\"");

I tend to favor the second option.

@joel-costigliola thoughts?

[joel-costigliola]

I like the second option better too because it directly uses the exception domain language (cause, root cause) whereas the extracting option shows implementation/plumbery on how to get cause and root cause which makes the code harder to read (and is not obvious to write)

[wimdeblauwe]

I like option 2 also better.

Maybe the naming could be containsCause() like the assert for Optional that has contains(String) ?

Or hasCauseSatisfying() like hasValueSatisfying of Optional?

Would it also mean that it would be possible to keep going down the exception hierarchy like this (using the naming of @scordio again for clarity):

assertThatExceptionOfType(PersistenceException.class)
    .isThrownBy(() -> {
        // test code here
    })
   .havingCause() // first cause
   .withMessageMatching("Key (user_id)=(.*) is not present in table \"book\"")
   .havingCause() // second cause
   .withMessageEndingWith("Foo");

I would then also add a havingRootCause to directly go to the root of the hierarchy as I think that is the most common use case.

[scordio]

My thinking of havingCause() is similar to Optional#get()

Would it also mean that it would be possible to keep going down the exception hierarchy like this

yes it would be possible to keep going down the hierarchy

I would then also add a havingRootCause to directly go to the root of the hierarchy as I think that is the most common use case.

I agree it would make sense

[joel-costigliola]

I was not explicit about it but I was thinking we should add both havingCause and havingRootCause.

[wimdeblauwe]

I am trying to see if I can create a PR to support this, but this is my first time navigating the code base, so I will need some guidance :)

I started with adding this to ThrowableAssertAlternative:

  public ThrowableAssertAlternative havingCause() {
    Throwable cause = delegate.actual.getCause();
    return new ThrowableAssertAlternative(cause);
  }

  public ThrowableAssertAlternative havingRootCause() {
    return new ThrowableAssertAlternative(Throwables.getRootCause(delegate.actual));
  }

But I wonder:
1) We should probably check if there is actually a cause (or root cause). What would be the best way to do this?
2) Should the return type be ThrowableAssertAlternative or ThrowableAssert (or something else)? Returning ThrowableAssertAlternative allows to chain calls of havingCause(), but there are no methods to check the message of the cause. Returning ThrowableAssert does have those methods, but then you can't chain another havingCause() on top.
3) What should be done with the generics of the return type?
4) There seem to be no unit tests for ThrowableAssertAlternative specifically?

I also noticed an import of org.assertj.core.util.Throwables which is only used in the Javadoc at lines 318 and 347. Isn't that wrong? I think:

the actual {@code Throwables}'s message.

should be:

the actual {@code Throwable}'s message.

[joel-costigliola]

We should probably check if there is actually a cause (or root cause). What would be the best way to do this?

We should indeed check for the cause/root cause, I would throw an AssertionError in that case with an explicit message like "expecting %s to have a cause/root cause but it did not". Something to add to ShouldHaveCause/ShouldHaveRootCause (with unit tests obviously 😉).

Should the return type be ThrowableAssertAlternative or ThrowableAssert?

If we started ThrowableAssertAlternative we should be consistent and continue with it.

Returning ThrowableAssertAlternative allows to chain calls of havingCause(), but there are no methods to check the message of the cause

Once you have called havingCause() the chained withMessage will check the message of the current exception which is the cause. Am I missing something here?

What should be done with the generics of the return type?

Since we can't know it, I think we should use ThrowableAssertAlternative<?>

There seem to be no unit tests for ThrowableAssertAlternative specifically?

An oversight, let's add some for the new methods (we will complete the tests but in a different PR)

[scordio]

@wimdeblauwe thanks for jumping on it! Feel free to open a pull request even if it's not fully ready and we can interact directly on top of your code.

I started with adding this to ThrowableAssertAlternative

--- edit --- my last proposal was not good from Generics point of view.

I also noticed an import of org.assertj.core.util.Throwables which is only used in the Javadoc at lines 318 and 347. Isn't that wrong?

That was definitely a typo, I've fixed it in 5339dc565b83db8c8835b870bca97aeec54e0ac9

[wimdeblauwe]

Something to add to ShouldHaveCause/ShouldHaveRootCause

Do you mean to add those methods to org.assertj.core.util.Throwables or AbstractThrowableAssert?
EDIT: You probably mean the classes like org.assertj.core.error.ShouldHaveCause and not new methods?

Once you have called havingCause() the chained withMessage will check the message of the current exception which is the cause. Am I missing something here?

No you are right, I had not looked closely enough :)

2 other questions after reading the contribution guidelines:

1. Should I fork the repo to open a PR? -> EDIT: I created a fork to be sure
2. The example test in the guidelines extends from BaseTest, but that class no longer exists and the test currently uses no base class at all. Should the guide be updated?

[wimdeblauwe]

See #1782 for my PR. I think/hope it is fairly ok. I did have to do something a bit strange in the ShouldHaveCause and ShouldHaveRootCause classes. As you cannot create a constructor with a single Throwable argument 2 times (for obvious reasons), I had to create an extra public factory method. If there is a better solution, please let me know.

[joel-costigliola]

@wimdeblauwe thanks for the PR, I'll have a look at it in the next few days.
Don't worry too much about the PR, we are here to give feedback and guidance.

The guidelines should be updated indeed, good catch!

[wimdeblauwe]

Shouln't the milestone 3.16.0 be attached to this issue?

[scordio]

Thanks, added 👍 We also forgot to introduce the @since in the javadoc, I added it in d4f191cdc2fa54a9f9d3cb7858a279142ce54090.