[POC] Migrate `JUnit` asserts to `AssertJ` - impl #2307

Pankraz76 · 2025-05-09T15:01:03Z

[POC] Migrate JUnit asserts to AssertJ - impl

tryout:

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.testing.assertj.JUnitToAssertj -Drewrite.exportDatatables=true

Pankraz76 · 2025-05-09T15:45:34Z

bug:

[junittoassertj] ReflectionValueExtractorTest.mappedIndexed:227 openrewrite/rewrite-testing-frameworks#728

[ERROR]   ReflectionValueExtractorTest.mappedIndexed:227 
expected: "a-value"
 but was: null

Pankraz76 · 2025-05-09T15:50:42Z

build success

Pankraz76 · 2025-05-09T16:12:21Z

bug:

missing dependency check: assertj-core openrewrite/rewrite-testing-frameworks#727

missing dependency check: assertj-core

      <dependency>
          <groupId>org.assertj</groupId>
          <artifactId>assertj-core</artifactId>
          <scope>test</scope>
      </dependency>

------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.0:testCompile (default-testCompile) on project maven-api-plugin: Compilation failure: Compilation failure: 
[ERROR] /Users/vincent.potucek/IdeaProjects/maven/api/maven-api-plugin/src/test/java/org/apache/maven/api/plugin/descriptor/another/ExtendedPluginDescriptorTest.java:[24,35] package org.assertj.core.api does not exist
[ERROR] /Users/vincent.potucek/IdeaProjects/maven/api/maven-api-plugin/src/test/java/org/apache/maven/api/plugin/descriptor/another/ExtendedPluginDescriptorTest.java:[24,1] static import only from classes and interfaces
[ERROR] /Users/vincent.potucek/IdeaProjects/maven/api/maven-api-plugin/src/test/java/org/apache/maven/api/plugin/descriptor/another/ExtendedPluginDescriptorTest.java:[74,9] cannot find symbol
[ERROR]   symbol:   method assertThat(java.lang.String)
[ERROR]   location: class org.apache.maven.api.plugin.descriptor.another.ExtendedPluginDescriptorTest
[ERROR] /Users/vincent.potucek/IdeaProjects/maven/api/maven-api-plugin/src/test/java/org/apache/maven/api/plugin/descriptor/another/ExtendedPluginDescriptorTest.java:[75,9] cannot find symbol
[ERROR]   symbol:   method assertThat(java.lang.String)
[ERROR]   location: class org.apache.maven.api.plugin.descriptor.another.ExtendedPluginDescriptorTest
[ERROR] -> [Help 1]

Pankraz76 · 2025-05-10T14:33:46Z

kindly request some feedback

gnodet

The PR contains unrelated commis.

Pankraz76 · 2025-05-11T08:52:12Z

The PR contains unrelated commis.

yes will squash.

This anyway just demo.

I would suggest to go with one single source of truth and not pay double the cost of carry and raising questions.

Where was the question about migrating to AssertJ risen and discussed already? Please provide me a link, I might have overseen it in my mails.

Migrate JUnit asserts to AssertJ - migration #2309 (comment)

Need to pitch and sync with community. Might be obvious that we benefit from strict diet, but of course all need to be heard, as SSOT is just an illusion and individual.

@Bukama @olamy

Pankraz76 · 2025-05-11T08:57:07Z

Illustration of New Code Already Being Out of Sync (Considered Outdated)

GitHub reference: apache/maven#2236 (comment)

JUnit vs AssertJ Comparison

Medium article reference

In summary:

JUnit: Testing framework for defining, executing, and reporting tests
AssertJ: Library that enhances assertion readability and expressiveness

Personal Experience

In my last project using JUnit5, we still maintained a modern assertThat structure.

StackOverflow reference about the assertThat method.

Proposal: Standardizing Test Style

Key points:

Adopt a single test style using the generic assertThat pattern
Both JUnit and AssertJ share this API design pattern
The actual framework implementation becomes an abstraction layer detail

Current problems:

Mixing assertTrue and assertThat creates inconsistency
No common base increases setup time
Requires recognizing different patterns for the same purpose

Goal:

Reduce complexity from 3 patterns to 1
Framework choice becomes a separate discussion

gnodet · 2025-05-11T09:24:54Z

impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java

-        assertNotNull(v);
-        assertEquals("", v.toString());
+        assertThat(v).isNotNull();
+        assertThat(v.toString()).isEqualTo("");


Is there any isEmpty() assertion ?

Is there any isEmpty() assertion ?

If not, you can create custom assertions for AssertJ very easly. I do this quite a lot at work for custom objects/classes

might be a bug, as failed to choose: https://docs.openrewrite.org/recipes/java/testing/assertj/simplifyassertjassertion

@timtebeek

Or simply need to chain.

Any change not standardized is weakened.

Simplest way would be JUnit Jupiter best practices and allow both.

[POC] JUnit Jupiter best practices #2320

or (continuous) migrate JUnit asserts to AssertJ like seen here.

Both will cure generic assertTrue statements elevate into assertThat.
Which lib is actually an random impl. detail.

gnodet · 2025-05-11T09:26:36Z

impl/maven-impl/src/test/java/org/apache/maven/impl/AbstractVersionTest.java

-            assertNotEquals(v2, v1, "expected " + v2 + " != " + v1);
+            assertThat(Integer.signum(v1.compareTo(v2))).as("expected " + v1 + " > " + v2).isEqualTo(1);
+            assertThat(Integer.signum(v2.compareTo(v1))).as("expected " + v2 + " < " + v1).isEqualTo(-1);
+            assertThat(v2).as("expected " + v1 + " != " + v2).isNotEqualTo(v1);


Aren't those messages automatically generated by assertJ in case of failures ?

gnodet

I think there's still room for improvements.
It would be nice if the details of the commit include a command used to refactor.

Pankraz76 · 2025-05-11T09:31:34Z

It would be nice if the details of the commit include a command used to refactor.

you mean this?

https://docs.openrewrite.org/recipes/java/testing/assertj/junittoassertj#usage

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.testing.assertj.JUnitToAssertj -Drewrite.exportDatatables=true

Pankraz76 · 2025-05-11T10:29:13Z

I think there's still room for improvements.

I would argue against this change now, realising and remembering that JUnit 5 looks almost 1o1 the same like AssertJ.

Both following and featuring assertThat prefix.

We could apply the API changes avoiding generic fluff assertTrue but use JUnit as its mandatory, which AssertJ might be not anymore.

Pankraz76 · 2025-05-11T10:37:39Z

JUnit 5 is equal, therefore superior to AssertJ.

only need to apply: JUnit Jupiter best practices

[POC] JUnit Jupiter best practices #2320

gnodet · 2025-05-11T16:36:07Z

JUnit 5 is equal, therefore superior to AssertJ.

only need to apply: JUnit Jupiter best practices

[POC] JUnit Jupiter best practices #2320

I'm actually skeptical.
The new ActivationFile().equals(null) actually calls the equals method and checks the result, whereas I assume the assertNotEquals or the asserts equivalent may bypass the call, just like Objects.equals if one of the parameters is null.

elharo

Hard no. I'm happy with JUnit and don't see a need to complicate matters and churn the code by using a different way to say the same things.

gnodet · 2025-05-12T20:54:42Z

Hard no. I'm happy with JUnit and don't see a need to complicate matters and churn the code by using a different way to say the same things.

There is a benefit: the message when an assertion fails is much more informative than with JUnit. I often have to debug and modify the jumit message to understand what the failure is about.
So I think it's a good idea.

Pankraz76 · 2025-05-12T21:04:13Z

different way to say the same things

yes, its nice to have same language, easy to understand each other.

benefit

This is your issue:

And fix:

Generic assertTrue is considered outdated resulting is broad fluff.

When following one framework, we should obey best practises. This will make every opinion around happy.

elharo · 2025-05-12T21:08:29Z

JUnit has a lot more than assertTrue. There's assertEquals, assertNull, assertSame, etc. I'm OK with things JUnit doesn't have like assertContains, but I don't want to replace things JUnit already has. Guava's MoreAsserts makes a lot more sense than AssertJ. It fills the potholes, not replaces the road.

Pankraz76 · 2025-05-13T07:50:02Z

MoreAsserts

I was wondering about what will be the next lib idk. There we have it.

Its a clear overhead now to follow one to rule them all. We should aim for SSOT.

Pankraz76 · 2025-05-13T10:55:18Z

might use what we already got; tighten things up, instead of going crazy. When done only once right, there is not need for rival, promoting overhead.

Going with most common default seems justiciable thus aligning with comment, supporting JUnit:

JUnit has a lot more than assertTrue. There's assertEquals, assertNull, assertSame, etc. I'm OK with things JUnit doesn't have like assertContains, but I don't want to replace things JUnit already has. Guava's MoreAsserts makes a lot more sense than AssertJ. It fills the potholes, not replaces the road.

[POC] rewrite-maven-plugin: Migrate to JUnit 5 from JUnit 4 #2330

Pankraz76 changed the title ~~Migrate JUnit asserts to AssertJ~~ Migrate JUnit asserts to AssertJ May 9, 2025

Pankraz76 mentioned this pull request May 9, 2025

Pull #2291: Modernize codebase with Java improvements - Unnecessary throws #2291

Closed

Pankraz76 mentioned this pull request May 9, 2025

[junittoassertj] ReflectionValueExtractorTest.mappedIndexed:227 openrewrite/rewrite-testing-frameworks#728

Closed

Pankraz76 marked this pull request as ready for review May 9, 2025 15:50

Pankraz76 changed the title ~~Migrate JUnit asserts to AssertJ~~ Migrate JUnit asserts to AssertJ - POC May 9, 2025

Pankraz76 mentioned this pull request May 9, 2025

Migrate JUnit asserts to AssertJ - migration #2309

Closed

Pankraz76 changed the title ~~Migrate JUnit asserts to AssertJ - POC~~ Migrate JUnit asserts to AssertJ - POC: impl/ May 9, 2025

This was referenced May 9, 2025

missing dependency check: assertj-core openrewrite/rewrite-testing-frameworks#727

Closed

Migrate JUnit asserts to AssertJ checkstyle/checkstyle#17047

Closed

Issue #17047: Migrate JUnit asserts to AssertJ checkstyle/checkstyle#17048

Closed

Pankraz76 marked this pull request as draft May 10, 2025 14:32

gnodet requested changes May 11, 2025

View reviewed changes

Pankraz76 mentioned this pull request May 11, 2025

[MNG-8686] Add SourceRoot.matcher(boolean) method #2236

Merged

Pankraz76 changed the title ~~Migrate JUnit asserts to AssertJ - POC: impl/~~ [POC] Migrate JUnit asserts to AssertJ - impl May 11, 2025

[POC] Migrate JUnit asserts to AssertJ - impl

432c7a3

Pankraz76 force-pushed the fix-write-fix-JUnitToAssertj branch from e3d5e17 to 432c7a3 Compare May 11, 2025 09:18

Pankraz76 requested a review from gnodet May 11, 2025 09:20

gnodet reviewed May 11, 2025

View reviewed changes

elharo requested changes May 12, 2025

View reviewed changes

Pankraz76 mentioned this pull request May 13, 2025

[POC] rewrite-maven-plugin: Migrate to JUnit 5 from JUnit 4 #2330

Closed

Pankraz76 closed this May 13, 2025

[POC] Migrate JUnit asserts to AssertJ - impl #2307

[POC] Migrate JUnit asserts to AssertJ - impl #2307

Uh oh!

Conversation

Pankraz76 commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented May 9, 2025

Uh oh!

Pankraz76 commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented May 10, 2025

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Pankraz76 commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pankraz76 commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Illustration of New Code Already Being Out of Sync (Considered Outdated)

JUnit vs AssertJ Comparison

Personal Experience

Proposal: Standardizing Test Style

Uh oh!

gnodet May 11, 2025

Choose a reason for hiding this comment

Uh oh!

Bukama May 11, 2025

Choose a reason for hiding this comment

Uh oh!

Pankraz76 May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pankraz76 May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnodet May 11, 2025

Choose a reason for hiding this comment

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Pankraz76 commented May 11, 2025

Uh oh!

Pankraz76 commented May 11, 2025

Uh oh!

Pankraz76 commented May 11, 2025

Uh oh!

gnodet commented May 11, 2025

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet commented May 12, 2025

Uh oh!

Pankraz76 commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elharo commented May 12, 2025

Uh oh!

Pankraz76 commented May 13, 2025

Uh oh!

Pankraz76 commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

[POC] Migrate `JUnit` asserts to `AssertJ` - impl #2307

[POC] Migrate `JUnit` asserts to `AssertJ` - impl #2307

Pankraz76 commented May 9, 2025 •

edited

Loading

Pankraz76 commented May 9, 2025 •

edited

Loading

Pankraz76 commented May 9, 2025 •

edited

Loading

Pankraz76 commented May 11, 2025 •

edited

Loading

Pankraz76 commented May 11, 2025 •

edited

Loading

Pankraz76 May 11, 2025 •

edited

Loading

Pankraz76 May 11, 2025 •

edited

Loading

Pankraz76 commented May 12, 2025 •

edited

Loading

Pankraz76 commented May 13, 2025 •

edited

Loading