review: refactor: update assertions to use AssertJ and improve readability #6578

MartinWitt · 2025-12-28T15:42:14Z

After fixing something in SpoonTestshelpers I was shocked to see Hamcrest usage

Copilot

Pull request overview

This PR refactors test assertions from Hamcrest to AssertJ and improves code readability by using Java text blocks and method references. The main changes include replacing Hamcrest matchers with AssertJ assertions, converting string concatenations to text blocks, and simplifying lambda expressions to method references.

Migrated test assertions from Hamcrest assertThat and matchers to AssertJ fluent assertions
Converted multi-line string concatenations to Java text blocks for improved readability
Simplified lambda expressions to method references where applicable

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 15 comments.

File	Description
src/test/java/spoon/test/SpoonTestHelpers.java	Converted `wrapLocal` method to use text block with formatted string
src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java	Replaced all Hamcrest assertions with AssertJ, converted string literal to text block, and simplified lambdas to method references

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-28T15:49:41Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		Assertions.assertThat(output).doesNotContain("import java.util.function.Function;");
+	}


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but this line uses tabs.

Copilot · 2025-12-28T15:49:42Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		Assertions.assertThat(output)
+				.contains(" MyClass")
+				.contains(" MyEnum")
+				.contains(" MyInterface");
+
+		// the code does not contain a 1, which would be the prefix of the local types' binary names
        // in the given code snippet
-        assertThat(output, not(containsString("1")));
+		Assertions.assertThat(output).doesNotContain("1");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but these lines use tabs.

Copilot · 2025-12-28T15:49:42Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		assertThat(sealedType).asString().contains("sealed");
        if (explicitPermitted.isEmpty()) {
            // the permits keyword should only exist if explicit permitted types are printed
-            assertThat(printed, not(containsString("permits")));
+			assertThat(sealedType).asString().doesNotContain("permits");
        } else {
-            assertThat(printed, containsString("permits"));
+			assertThat(sealedType).asString().contains("permits");
            for (String permitted : explicitPermitted) {
-                assertThat(printed, containsRegexMatch("\\s" + permitted));
+				assertThat(sealedType).asString().containsPattern("\\s" + permitted);


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but these lines use tabs.

Copilot · 2025-12-28T15:49:42Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

        CtClass<?> ctClass = launcher.getFactory().Class().create("MyClass");
        ctClass.addModifier(ModifierKind.NON_SEALED);
-        assertThat(ctClass.toString(), containsString("non-sealed "));
+		assertThat(ctClass).asString().contains("non-sealed ");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but this line uses tabs.

Copilot · 2025-12-28T15:49:42Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		Assertions.assertThat(spoonModelBuilder.build()).isTrue();

        CtLocalVariable<Integer> localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1);
-        assertThat(localVariable.toString(), equalTo("int fieldReadOfA = ((A) c).a.i"));
+		assertThat(localVariable).asString().isEqualTo("int fieldReadOfA = ((A) c).a.i");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but these lines use tabs.

Copilot · 2025-12-28T15:49:44Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		assertThat(x).asString().contains("Set<T>", "List<T> list");
+
+		assertThat(x).asString().containsPattern("Collection<.*String>");
+		assertThat(x).asString().containsPattern("List<.*List<T>>");
+		assertThat(x).asString().containsPattern("List<.*List<\\? extends T>>");
+		assertThat(x).asString().containsPattern("List<.*List<\\? super T>>");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but these lines use tabs.

Copilot · 2025-12-28T15:49:44Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

+		Assertions.assertThat(spoonModelBuilder.build()).isTrue();

        CtLocalVariable<Integer> localVariable = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(0);
-        assertThat(localVariable.toString(), equalTo("int myInt = (int) myDouble"));
+		assertThat(localVariable).asString().isEqualTo("int myInt = (int) myDouble");

        CtLocalVariable<Integer> localVariable2 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(1);
-        assertThat(localVariable2.toString(), equalTo("int myInt2 = ((java.lang.Double) myDouble).intValue()"));
+		assertThat(localVariable2).asString().isEqualTo("int myInt2 = ((java.lang.Double) myDouble).intValue()");

        CtLocalVariable<Integer> localVariable3 = model.getElements(new TypeFilter<>(CtLocalVariable.class)).get(2);
-        assertThat(localVariable3.toString(), equalTo("double withoutTypeCast = myDoubleObject.doubleValue()"));
+		assertThat(localVariable3).asString().isEqualTo("double withoutTypeCast = myDoubleObject.doubleValue()");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but these lines use tabs.

Copilot · 2025-12-28T15:49:44Z

src/test/java/spoon/test/SpoonTestHelpers.java

+		return """
+			class X {
+			  public void myMethod() {
+			  %s


The text block format adds unwanted indentation before the substituted content. The original code inserted localDeclarationSnippet directly without any leading spaces, but the new text block has two spaces before %s on line 184. This will add two spaces of indentation to all content passed to this method, which changes the behavior of the function.

Suggested change

%s

%s

Copilot · 2025-12-28T15:49:45Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

        complexTypeReference.addActualTypeArgument(complexTypeReference.clone().setSimpleName("Comhyperbola"));
        exeExpression.getExecutable().addActualTypeArgument(complexTypeReference);
-        assertThat(exeExpression.toString(), containsRegexMatch("<(.*)Comparable<(.*)Integer, (.*)Comhyperbola<(.*)Integer>>>"));
+		assertThat(exeExpression).asString().containsPattern("<(.*)Comparable<(.*)Integer, (.*)Comhyperbola<(.*)Integer>>>");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but this line uses tabs.

Copilot · 2025-12-28T15:49:45Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

-
            // assert
-            assertThat(actualStringRepresentation, equalTo("int[] intArray = new int[]{ 3 }"));
+			assertThat(localVariable).asString().isEqualTo("int[] intArray = new int[]{ 3 }");


The indentation uses tabs instead of spaces, which is inconsistent with the rest of the file. The file uses spaces for indentation (4 spaces per level), but this line uses tabs.

…st.java Co-authored-by: Copilot <[email protected]>

SirYwell · 2025-12-31T09:32:36Z

Are there more hamcrest usages around somewhere else? In best case, we could remove it at some point.

MartinWitt · 2025-12-31T11:46:49Z

Are there more hamcrest usages around somewhere else? In best case, we could remove it at some point.

yes but this would end in a monster PR, so I prefer splitted.

MartinWitt · 2025-12-31T13:48:20Z

Okay looks like we could run mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.testing.hamcrest.MigrateHamcrestToAssertJ -Drewrite.exportDatatables=true and most of hamcrest is gone.

I-Al-Istannen · 2026-01-02T13:33:27Z

Okay looks like we could run mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.testing.hamcrest.MigrateHamcrestToAssertJ -Drewrite.exportDatatables=true and most of hamcrest is gone.

That would probably be nice as a second PR?

I-Al-Istannen · 2026-01-02T13:34:08Z

src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTest.java

-            // act
-            String actualStringRepresentation = localVariable.toString();


I think keeping these might be worthwhile for clarity? There are 2-3 of those you inlined into the assert. But no hard feelings from my side.

monperrus · 2026-01-18T04:22:22Z

thanks @MartinWitt

refactor: update assertions to use AssertJ and improve readability

b45ab6b

Copilot AI review requested due to automatic review settings December 28, 2025 15:42

Copilot started reviewing on behalf of MartinWitt December 28, 2025 15:42 View session

Copilot AI reviewed Dec 28, 2025

View reviewed changes

MartinWitt and others added 4 commits December 28, 2025 16:51

Update src/test/java/spoon/reflect/visitor/DefaultJavaPrettyPrinterTe…

c6b830d

…st.java Co-authored-by: Copilot <[email protected]>

remove unused import

5023f3e

to tabs

9a5094f

refactor: replace JUnit assertions with AssertJ for improved readability

7daf86a

MartinWitt changed the title ~~refactor: update assertions to use AssertJ and improve readability~~ review: refactor: update assertions to use AssertJ and improve readability Dec 29, 2025

MartinWitt requested a review from SirYwell December 29, 2025 13:30

I-Al-Istannen reviewed Jan 2, 2026

View reviewed changes

monperrus merged commit 7da1872 into master Jan 18, 2026
16 checks passed

		Assertions.assertThat(output).doesNotContain("import java.util.function.Function;");
		}

		// act
		String actualStringRepresentation = localVariable.toString();

Uh oh!

review: refactor: update assertions to use AssertJ and improve readability #6578

review: refactor: update assertions to use AssertJ and improve readability #6578

Conversation

MartinWitt commented Dec 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

SirYwell commented Dec 31, 2025

Uh oh!

MartinWitt commented Dec 31, 2025

Uh oh!

MartinWitt commented Dec 31, 2025

Uh oh!

I-Al-Istannen commented Jan 2, 2026

Uh oh!

I-Al-Istannen Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

monperrus commented Jan 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants