Sonarcloud is now reporting “Complete assertion” for assertThatThrownBy(). While it’s true that plain assertThat() and possibly assertThatCode() make no implicit assertions, that is not true for assertThatThrownBy(); the latter will fail if the invoked lambda expression does not throw an exception. Lots of test code makes use of this fact to verify that the code throws an exception.
Work-around: add “.instanceOf(Exception.class)” to the end of each assertThatThrownBy() invocation.
Welcome the community and happy to see you are using the recent rules we released about AssertJ!
I believe you should be explicit about which exception you expect to be raised for each call to assertThatThrownBy(), that’s the goal of this assertion otherwise the next developer who will work on that code will not know why you wrote that line.
If you are using assertThatThrownBy() just in case an exception is raised, I’m not sure you use it for what this assertion was made.
It is not used “just in case an exception is raised”. As the method name implies, it is used to ensure that an exception is raised. It supports additional methods if the tester would prefer the tests to be more specific, but sonar rules should not require their use. I’m pretty sure that anyone that sees an asssertThatThrownBy() knows exactly why that line of code was written - to ensure that the lambda throws an exception.
When I write a test for something that throws an exception, I generally include an isInstanceOf() or some other method to ensure the exception is of the type or content expected. However, some code just passes generic exceptions from underlying code; there isn’t much point in adding an isInstanceOf(Exception.class) to that - all I, as the tester care, is that it does indeed throw an exception.
Keep in mind, this method is not the same as assertThat() or assertThatCode(), which appear to make minimal, if any, checks unless explicitly specified.
Thanks to the team for taking the time to consider this and thanks for letting me know the outcome of your decision. Interestingly, while it was a nuisance to make the change, I had decided that your argument was compelling enough that I went back and added isInstanceOf(ExpectedException.class) to most of them.
