You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by "Marcono1234 (Jira)" <ji...@apache.org> on 2023/06/03 13:10:00 UTC

[jira] [Created] (FLINK-32248) Incorrect AssertJ usage

Marcono1234 created FLINK-32248:
-----------------------------------

             Summary: Incorrect AssertJ usage
                 Key: FLINK-32248
                 URL: https://issues.apache.org/jira/browse/FLINK-32248
             Project: Flink
          Issue Type: Bug
          Components: Tests
            Reporter: Marcono1234


Flink uses AssertJ incorrectly:
- Multiple tests callĀ AssertJ's {{withFailMessage}} at the end, for example something like this:
{code}
assertThat(...)
    ...()
    .withFailMessage("Some message");
{code}
However, the [{{withFailMessage}} documentation|https://www.javadoc.io/doc/org.assertj/assertj-core/latest/org/assertj/core/api/AbstractAssert.html#withFailMessage(java.lang.String,java.lang.Object...)] says:
{quote}
You must set it *before* calling the assertion otherwise it is ignored as the failing assertion breaks the chained call by throwing an AssertionError. 
{quote}
This can be seen for example with the following code where the custom message is ignored:
{code}
 assertThat(false).isTrue().withFailMessage("Some message");
{code}
There are 20+ cases of this in the Flink code so I am not going to list them here, hoping that it is easier to find them yourself. Though please let me know if I should list them here.

- No assertion being performed by {{WebFrontendITCase}}. There are (currently) three cases where accidentally no call is made on the {{assertThat}} return value. For example something like this:
{code}
assertThat(list.isEmpty());
// Should instead be
assertThat(list).isEmpty();
{code}
-- https://github.com/apache/flink/blob/bfe49b2973d4ffc8f7404a376cab1e419b53406a/flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java#L302-L302
-- https://github.com/apache/flink/blob/bfe49b2973d4ffc8f7404a376cab1e419b53406a/flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java#L372-L372
-- https://github.com/apache/flink/blob/bfe49b2973d4ffc8f7404a376cab1e419b53406a/flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java#L416-L416

I found those because AssertJ is luckily using {{@CheckReturnValue}} on their API. I assume SpotBugs, Error Prone or IntelliJ IDEA might have found these issues as well, in case you want to investigate integrating automatic detection of such errors.




--
This message was sent by Atlassian Jira
(v8.20.10#820010)