Move Junit 5 assertThrows to last statement in lamdba

[shivanisky]

What's changed?

In multi line assertThrows move assertThrows to last statement only

What's your motivation?

Currently the Junit 4 to 5 recipe migrates the @test junit 4 annotation to wrap the entire method to assertThrows when typically we want it on the last statement.

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

Yes - may be modifying the existing recipe is better? Rather than providing an extra recipe to move existing assertThrows with multi line lamdbas to the last statement only, but we can hook this recipe in optionally, so ... maybe not. Maybe that can be an optional argument in the existing recipe - move applyAssertThrowsToLast and the user can set it to true if they are ok to take a hit of compilation errors in favour of better usage of junit 5 features and correctness.

Any additional context

• JUnit 5 Assertions.assertThrows should only contain a single statement #194

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[timtebeek]

Hi @shivanisky ; Great start! I've pushed a few quick changes to get you on a path to get this mostly working; then from here we'd have to guard against the cases where we might do harm, which would be the more interesting challenge of this recipe. There's a few such cases described in #194 that we might want to cover here. Thanks again for exploring if we an actually see this through in a mostly reliable way.

[shivanisky]

Hi @shivanisky ; Great start! I've pushed a few quick changes to get you on a path to get this mostly working; then from here we'd have to guard against the cases where we might do harm, which would be the more interesting challenge of this recipe. There's a few such cases described in #194 that we might want to cover here. Thanks again for exploring if we an actually see this through in a mostly reliable way.

Thank you so much for your optimisations and feature adds! Its looking very close now :).

Regarding the final/non final variable it seems it's a bit overkill to do as it's a rare case?

I'm thinking, what if we hook this into the main junit 4 to 5 recipe which migrates the assertThrows if an optional parameter is specified in yml file to run it? I would like to do it this way.

[shivanisky]

Added case for variable declaration ie Throwable ex = assertThrows(....) seems to be working well.

[shivanisky]

I think no point in putting a lot of effort for non final variables case - I am not seeing it across multiple repos with thousands of test cases and over 500 files. Have put in warning in description regarding this rare case, and I think better that people can use it if they want to. Eventually, can hook in to the main junit 5 migration recipe with argument to optionally use this, so that scope is limited to migrating the junit 4 @test annotation only rather than touching existing assertThrows lamdas.