You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Pankraz76 (via GitHub)" <gi...@apache.org> on 2025/05/09 15:01:06 UTC
[PR] Migrate JUnit asserts to AssertJ [maven]
Pankraz76 opened a new pull request, #2307:
URL: https://github.com/apache/maven/pull/2307
why having 2 if one is enough?
https://docs.openrewrite.org/recipes/java/testing/assertj/junittoassertj
I always had ether or but never the need to pay cost of carry twice for same concern.
<img width="897" alt="image" src="https://github.com/user-attachments/assets/a55dc800-0406-4a7c-84c4-8ebdda7c3a54" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` - `POC: impl/` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869637956
> 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.
- https://github.com/apache/maven/pull/2309#issuecomment-2867799299
Need to pitch and sync with community. Might be obvious that we need to act, but of course all need to be heared.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083549961
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
might be a bug, as failed to choose: https://docs.openrewrite.org/recipes/java/testing/assertj/simplifyassertjassertion
@timtebeek
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` - `POC: impl/` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2867117890
bug
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]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083464719
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
Is there any `isEmpty()` assertion ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 closed pull request #2307: [POC] Migrate `JUnit` asserts to `AssertJ` - impl
URL: https://github.com/apache/maven/pull/2307
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083549961
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
might be a bug, as failed to choose: https://docs.openrewrite.org/recipes/java/testing/assertj/simplifyassertjassertion
@timtebeek
Or simply need to chain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869970640
> JUnit 5 is equal, therefore superior to AssertJ.
>
>
>
> only need to apply: JUnit Jupiter best practices
>
>
>
> - https://github.com/apache/maven/pull/2320
>
>
>
> <img width="1525" alt="image" src="https://github.com/user-attachments/assets/0cd8121c-ba09-44af-b408-604711e7817b" />
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869679005
JUnit 5 is equal, therefore superior to AssertJ.
only need to apply: JUnit Jupiter best practices
- https://github.com/apache/maven/pull/2320
<img width="1525" alt="image" src="https://github.com/user-attachments/assets/0cd8121c-ba09-44af-b408-604711e7817b" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Bukama (via GitHub)" <gi...@apache.org>.
Bukama commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083465266
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2867056212
bug:
[ERROR] ReflectionValueExtractorTest.mappedIndexed:227
expected: "a-value"
but was: null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083552318
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
Any change not standardized is weakened.
Simplest way would be `JUnit Jupiter best practices` and allow both.
- https://github.com/apache/maven/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` - `POC: impl/` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2868934873
kindly request some feedback
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2874024591
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2876015707
might use what we already got making things tight, instead of going crazy. When done only once right, we dont need rival. Going with most common default seems justiciable thus aligning with:
> 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.
- https://github.com/apache/maven/pull/2330
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083465179
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/AbstractVersionTest.java:
##########
@@ -40,21 +39,21 @@ protected void assertOrder(int expected, String version1, String version2) {
Version v2 = newVersion(version2);
if (expected > 0) {
- assertEquals(1, Integer.signum(v1.compareTo(v2)), "expected " + v1 + " > " + v2);
- assertEquals(-1, Integer.signum(v2.compareTo(v1)), "expected " + v2 + " < " + v1);
- assertNotEquals(v1, v2, "expected " + v1 + " != " + v2);
- 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);
Review Comment:
Aren't those messages automatically generated by assertJ in case of failures ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869675626
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2874068572
> different way to say the same things
its nice to have same language, easy understanding each other.
https://www.opslevel.com/resources/standards-in-software-development-and-9-best-practices
> benefit
This is your issue:
- https://eliasnogueira.com/dont-use-asserttrue-to-verify-a-text/
- https://dheerajgambhir.medium.com/asserttrue-vs-assertequals-for-text-verification-c39cc0d27cdd
And fix:
- https://docs.openrewrite.org/recipes/java/testing/junit5/junit5bestpractices
- https://docs.openrewrite.org/recipes/java/testing/junit5/cleanupassertions#example-2
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` - `POC: impl/` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869639674
mentioning illustration of new code, already being out of sync, even considered outdated:
- https://github.com/apache/maven/pull/2236#discussion_r2083456022
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2875418983
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2874086355
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2869654265
> 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`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [POC] Migrate `JUnit` asserts to `AssertJ` - impl [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2307:
URL: https://github.com/apache/maven/pull/2307#discussion_r2083552318
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultModelVersionParserTest.java:
##########
@@ -22,15 +22,14 @@
import org.eclipse.aether.util.version.GenericVersionScheme;
import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.assertj.core.api.Assertions.assertThat;
class DefaultModelVersionParserTest {
@Test
void parseVersion() {
Version v = new DefaultModelVersionParser(new GenericVersionScheme()).parseVersion("");
- assertNotNull(v);
- assertEquals("", v.toString());
+ assertThat(v).isNotNull();
+ assertThat(v.toString()).isEqualTo("");
Review Comment:
Any change not standardized is weakened.
Simplest way would be `JUnit Jupiter best practices` and allow both.
- https://github.com/apache/maven/pull/2320
or (continuous) migrate `JUnit` asserts to `AssertJ` like seen here.
Both will cure generic `assertTrue` statements resulting in `assertThat`. Which `lib` is actually an random impl. detail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate `JUnit` asserts to `AssertJ` [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on PR #2307:
URL: https://github.com/apache/maven/pull/2307#issuecomment-2867066640
build success
<img width="1347" alt="image" src="https://github.com/user-attachments/assets/14cafcbd-586f-464c-8969-bfb102afde37" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org