Doing a code review, I ran into this assertion in a unit test:

        assertThatThrownBy(() -> shoppingCartService.payForCart(command))
            .isInstanceOfSatisfying(PaymentException.class,
                                    exception -> assertThat(exception.getMessage())
                                        .isEqualTo("Cannot pay for ids [" + item.getId() +
                                                        "], status is not WAITING"));

I feel that testing the literal text of an Exception is bad practice, but I'm unable to convince the author. (Note: this is not localized text; it's a String literal in the code with the item ID as a parameter.) My reasoning:

• If we decided that, e.g. the item's ID was a critical piece of information, it might make sense to test that the message contains the ID. But in this case, it would be better to include the ID as a field of the Exception class.
• If we had some sort of external system that was automatically reading the logs and looking for certain strings (we don't), then yes, this test would be justified
• What's important in the message is that it clearly communicates what the problem is. But there must be hundreds of ways of constructing such a message, and no testing library can tell us if a plain text human language String is "clear" or "useful".
• Thus, this amounts to, e.g. writing unit tests for translations - it's pointless because the best you can do boils down to duplicating your messages file in the tests.

What's the best practice here, and why?

Doing a code review, I ran into this assertion in a unit test:

assertThatThrownBy(() -> shoppingCartService.payForCart(command))
  .isInstanceOfSatisfying(PaymentException.class,  
    exception -> assertThat(exception.getMessage()).isEqualTo(
      "Cannot pay for ids [" + item.getId() +"], status is not WAITING"));

I feel that testing the literal text of an Exception is bad practice, but I'm unable to convince the author. (Note: this is not localized text; it's a String literal in the code with the item ID as a parameter.) My reasoning:

• If we decided that, e.g. the item's ID was a critical piece of information, it might make sense to test that the message contains the ID. But in this case, it would be better to include the ID as a field of the Exception class.
• If we had some sort of external system that was automatically reading the logs and looking for certain strings (we don't), then yes, this test would be justified
• What's important in the message is that it clearly communicates what the problem is. But there must be hundreds of ways of constructing such a message, and no testing library can tell us if a plain text human language String is "clear" or "useful".
• Thus, this amounts to, e.g. writing unit tests for translations - it's pointless because the best you can do boils down to duplicating your messages file in the tests.

What's the best practice here, and why?