Refactor @Test(expected) with assertThrows

[Taher-Ghaleb]

I am working on research that investigates test smell refactoring in which we identify alternative implementations of test cases, study how commonly used these refactorings are, and assess how acceptable they are in practice.

For example, the smell in SlotSharingGroupTest.java occurs when exception handling can alternatively be implemented using assertion rather than annotation: using assertThrows(IllegalArgumentException.class, () -> {...}); instead of @Test(expected = IllegalArgumentException.class).

While there are many cases like this, we aim in this pull request to get your feedback on this particular test smell and its refactoring. Thanks in advance for your input.

[flinkbot]

CI report:

• 180baf6 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[Taher-Ghaleb]

Hi, any updates or feedback on the changes, please? Thanks.

[X-czh]

Hi @Taher-Ghaleb, thanks for your notice on Flink. If you would like to contribute to Flink, you should follow the guideline here, where the first step is to create a JIRA issue and discuss to reach consensus. Also, for the problem you noticed, the community has started the shift from JUnit4 to JUnit5 + AssertJ a while ago and will address the issue you raised here with AssertJ's assertThatThrownBy, see this JIRA ticket. However, due to the huge size of the codebase, the shift may take some extra months to finish. If you'd like to contribute, please ask committers to get one of the ticket assigned to you.

[X-czh]

With that in mind, I think we should close this MR for now. WDYT?

[Taher-Ghaleb]

Thanks @X-czh for your response. The purpose of this pull request was to get insights about why such cases as exception handling still exist in codebases though there exist better alternatives. Your response indicates that you are already aware of this issue and have taken steps to improve the quality of test cases.

[X-czh]

Got it, thanks @Taher-Ghaleb for raising the discussion here. Seems like consensus is already reached, can we close this MR then?