rewrite-testing-frameworks
rewrite-testing-frameworks copied to clipboard
Collapse consecutive AssertJ `assertThat(..)` statements that each use the same object to a single statement
What problem are you trying to solve?
What precondition(s) should be checked before applying this recipe?
Only consecutive calls; not when interleaved with others.
Only when the asserted object does not change; so for instance no calls to extracting("baz")
, as that would change the semantics.
Describe the situation before applying the recipe
assertThat(listA).isNotNull();
assertThat(listA).hasSize(3);
assertThat(listA).containsExactly("a", "b", "c");
Describe the situation after applying the recipe
assertThat(listA)
.isNotNull()
.hasSize(3)
.containsExactly("a", "b", "c");
Any additional context
Would help clean up after the migration from Hamcrest allOf(Matcher...)
to individual statements.
Perhaps @pstreef can provide some pointers on the implementation here if he's willing to share, as I know he's done some similar work recently in an unrelated project. To be sure: I don't expect him to pick this up, but the general approach of collapsing subsequent calls to a more fluent invocation seems familiar. :)
Sure thing @timtebeek!
In most of the cases these assertions happen in the same method (maybe only work on @Test
methods) and on either on a local variable, global variable or method invocation.
I would only focus on local variables to start with, and maybe extend to global if it makes sense. Method invocation should not be considered as the return values do not have to always be the same for subsequent calls.
I would only take into account calls that are consecutive, because if there is any other statement in between that could change the variable being asserted:
String a = "a";
assertThat(a).isNotNull();
a = "b";
assertThat(a).isEqualTo("a");
To get starter you will either have to start from a visitBlock(..)
or visitMethodDeclaration(..)
(if you want to filter on test methods, the later is helpful), itterate over all statements in the block/body and find any statement where a J.MethodInvocation
has a select that is itself a J.MethodInvocation
to assertThat(..)
Alternatively you can start from visitMethodInvocation(..)
and find any J.MethodInvocation
that has a select that is a J.MethodInvocation
to assertThat(..)
. However that makes it harder to determine if there is only 1 call after assertThat(x)
which might cause trouble.
Make sure to:
- Only collect them if they are consecutive and the
assertThat(x)
call have exactly the same parameters. - Only collect if the chained calls do not change semantics
- Dont collect calls that already are chaining more than once
Then chain those calls by appending each chained call to the next one and ending with the asserThat(x)
using withSelect(..)
Edge cases to test for:
- non local variable arguments
- non consecutive calls
- calls with different arguments
- calls with methodInvocations as argument
- calls where the semantics change ( for instance
extracting
) - multiple already chained calls
- calls made in nested blocks