elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1231081124
########## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ########## @@ -147,10 +154,10 @@ void testVersionsEqual() { checkVersionsEqual("1.0.0x", "1-x"); // aliases - checkVersionsEqual("1ga", "1"); - checkVersionsEqual("1release", "1"); - checkVersionsEqual("1final", "1"); - checkVersionsEqual("1cr", "1rc"); + checkVersionsEqualOrder("1ga", "1"); + checkVersionsEqualOrder("1release", "1"); + checkVersionsEqualOrder("1final", "1"); Review Comment: OK, I think I understand you now. The behavior does change. The use of checkVersonsEquals and checkVersionsEqualOrder hides this though. Consider adding a test that the versions are no equalk. ########## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ########## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } + private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: Yes, what's unreadable is that I don't know which is greater and which is lesser. Or if it's doing something else then I am being confused by the name. Perhaps a clearer name would help, but it'ds also OK to just inline it. ########## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ########## @@ -383,4 +390,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + + @Test + public void testMng7714() { + String f = "1.0.final-redhat"; + String sp1 = "1.0-sp1-redhat"; + String sp2 = "1.0-sp-1-redhat"; + String sp3 = "1.0-sp.1-redhat"; + checkVersionsOrder(f, sp1); Review Comment: only new tests please. Do not change existing tests unless the behavior has changed. -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org