From 3adea80fd5b02e5720abed6ae24ca77e2a16c8b5 Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Mon, 1 Sep 2025 16:48:03 +0200 Subject: [PATCH 1/2] Add self-tests for `assert[Not]Equals()` with arrays --- .../org/junit/jupiter/api/AssertionUtils.java | 8 +++++ .../api/AssertEqualsAssertionsTests.java | 35 +++++++++++++++++++ .../api/AssertNotEqualsAssertionsTests.java | 19 ++++++++++ 3 files changed, 62 insertions(+) diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java index 6ae0c48536a7..ad1ec00298f1 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java @@ -18,6 +18,8 @@ import org.jspecify.annotations.Nullable; import org.junit.platform.commons.annotation.Contract; +import org.junit.platform.commons.logging.Logger; +import org.junit.platform.commons.logging.LoggerFactory; import org.junit.platform.commons.util.UnrecoverableExceptions; /** @@ -28,6 +30,8 @@ */ class AssertionUtils { + private static final Logger logger = LoggerFactory.getLogger(AssertionUtils.class); + private AssertionUtils() { /* no-op */ } @@ -129,6 +133,10 @@ static boolean objectsAreEqual(@Nullable Object obj1, @Nullable Object obj2) { if (obj1 == null) { return (obj2 == null); } + if (obj1.getClass().isArray() && (obj2 != null && obj2.getClass().isArray())) { + // TODO Find first method in user's code, i.e. non-framework code. + logger.debug(() -> "Should have used `assertArrayEquals()` in method: "); + } return obj1.equals(obj2); } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java index ec30168e7605..8612e5ff6444 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java @@ -13,12 +13,18 @@ import static org.junit.jupiter.api.AssertionTestUtils.assertExpectedAndActualValues; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEndsWith; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEquals; +import static org.junit.jupiter.api.AssertionTestUtils.assertMessageMatches; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageStartsWith; import static org.junit.jupiter.api.AssertionTestUtils.expectAssertionFailedError; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertLinesMatch; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.logging.LogRecord; + +import org.junit.jupiter.api.fixtures.TrackLogRecords; import org.junit.jupiter.api.function.Executable; +import org.junit.platform.commons.logging.LogRecordListener; import org.opentest4j.AssertionFailedError; /** @@ -750,6 +756,35 @@ void chars() { } + @Nested + class ArraysAsArguments { + @Test + void objects(@TrackLogRecords LogRecordListener listener) { + Object object = new Object(); + Object array1 = new Object[] { object }; + Object array2 = new Object[] { object }; + try { + assertEquals(array1, array2); + expectAssertionFailedError(); + } + catch (AssertionFailedError ex) { + assertMessageMatches(ex, "expected: " + // + "\\Q[Ljava.lang.Object;@\\E" + // + ".+" + // + "\\Q<[java.lang.Object@\\E" + // + ".+" + // + "\\Q]> but was: [Ljava.lang.Object;@\\E" + // + ".+" + // + "\\Q<[java.lang.Object@\\E" + // + ".+" + // + "\\Q]>\\E"); + } + assertLinesMatch(""" + Should have used `assertArrayEquals()` in method: + """.lines(), listener.stream(AssertionUtils.class).map(LogRecord::getMessage)); + } + } + // ------------------------------------------------------------------------- @SuppressWarnings("overrides") diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java index 842e7f463a8f..48a55768ded4 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java @@ -14,9 +14,14 @@ import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEquals; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageStartsWith; import static org.junit.jupiter.api.AssertionTestUtils.expectAssertionFailedError; +import static org.junit.jupiter.api.Assertions.assertLinesMatch; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.logging.LogRecord; + +import org.junit.jupiter.api.fixtures.TrackLogRecords; +import org.junit.platform.commons.logging.LogRecordListener; import org.opentest4j.AssertionFailedError; /** @@ -624,6 +629,20 @@ void assertNotEqualsInvokesEqualsMethodForIdenticalObjects() { } + @Nested + class AssertNotEqualsArrays { + @Test + void objects(@TrackLogRecords LogRecordListener listener) { + Object object = new Object(); + Object array1 = new Object[] { object }; + Object array2 = new Object[] { object }; + assertNotEquals(array1, array2); + assertLinesMatch(""" + Should have used `assertArrayEquals()` in method: + """.lines(), listener.stream(AssertionUtils.class).map(LogRecord::getMessage)); + } + } + // ------------------------------------------------------------------------- @Nested From 9917f9c0a8f07a0430e402c3b4b5b47aebdce0cd Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Wed, 17 Dec 2025 10:22:51 +0100 Subject: [PATCH 2/2] Use system property flag Signed-off-by: Christian Stein --- .../org/junit/jupiter/api/AssertionUtils.java | 17 +++++----- .../api/AssertEqualsAssertionsTests.java | 31 +++++-------------- .../api/AssertNotEqualsAssertionsTests.java | 30 ++++++++++++------ 3 files changed, 38 insertions(+), 40 deletions(-) diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java index ad1ec00298f1..05165b58e83c 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java @@ -17,9 +17,8 @@ import java.util.function.Supplier; import org.jspecify.annotations.Nullable; +import org.junit.platform.commons.JUnitException; import org.junit.platform.commons.annotation.Contract; -import org.junit.platform.commons.logging.Logger; -import org.junit.platform.commons.logging.LoggerFactory; import org.junit.platform.commons.util.UnrecoverableExceptions; /** @@ -29,9 +28,6 @@ * @since 5.0 */ class AssertionUtils { - - private static final Logger logger = LoggerFactory.getLogger(AssertionUtils.class); - private AssertionUtils() { /* no-op */ } @@ -133,9 +129,14 @@ static boolean objectsAreEqual(@Nullable Object obj1, @Nullable Object obj2) { if (obj1 == null) { return (obj2 == null); } - if (obj1.getClass().isArray() && (obj2 != null && obj2.getClass().isArray())) { - // TODO Find first method in user's code, i.e. non-framework code. - logger.debug(() -> "Should have used `assertArrayEquals()` in method: "); + if (obj2 == null) { + return false; + } + if (Boolean.getBoolean("junit.jupiter.disallow.arrays.in.equals.checks")) { + if (obj1.getClass().isArray() && obj2.getClass().isArray()) { + throw new JUnitException( + "Detected array arguments:" + " " + obj1.getClass() + " and " + obj2.getClass()); + } } return obj1.equals(obj2); } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java index 8612e5ff6444..884952353cd0 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertEqualsAssertionsTests.java @@ -13,18 +13,13 @@ import static org.junit.jupiter.api.AssertionTestUtils.assertExpectedAndActualValues; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEndsWith; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEquals; -import static org.junit.jupiter.api.AssertionTestUtils.assertMessageMatches; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageStartsWith; import static org.junit.jupiter.api.AssertionTestUtils.expectAssertionFailedError; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertLinesMatch; import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.logging.LogRecord; - -import org.junit.jupiter.api.fixtures.TrackLogRecords; import org.junit.jupiter.api.function.Executable; -import org.junit.platform.commons.logging.LogRecordListener; +import org.junit.platform.commons.JUnitException; import org.opentest4j.AssertionFailedError; /** @@ -759,29 +754,19 @@ void chars() { @Nested class ArraysAsArguments { @Test - void objects(@TrackLogRecords LogRecordListener listener) { + void objects() { Object object = new Object(); Object array1 = new Object[] { object }; Object array2 = new Object[] { object }; try { - assertEquals(array1, array2); - expectAssertionFailedError(); + System.setProperty("junit.jupiter.disallow.arrays.in.equals.checks", "true"); + var exception = assertThrows(JUnitException.class, () -> assertEquals(array1, array2)); + assertEquals("Detected array arguments: class [Ljava.lang.Object; and class [Ljava.lang.Object;", + exception.getMessage()); } - catch (AssertionFailedError ex) { - assertMessageMatches(ex, "expected: " + // - "\\Q[Ljava.lang.Object;@\\E" + // - ".+" + // - "\\Q<[java.lang.Object@\\E" + // - ".+" + // - "\\Q]> but was: [Ljava.lang.Object;@\\E" + // - ".+" + // - "\\Q<[java.lang.Object@\\E" + // - ".+" + // - "\\Q]>\\E"); + finally { + System.clearProperty("junit.jupiter.disallow.arrays.in.equals.checks"); } - assertLinesMatch(""" - Should have used `assertArrayEquals()` in method: - """.lines(), listener.stream(AssertionUtils.class).map(LogRecord::getMessage)); } } diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java index 48a55768ded4..15ab15226f14 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/api/AssertNotEqualsAssertionsTests.java @@ -14,14 +14,16 @@ import static org.junit.jupiter.api.AssertionTestUtils.assertMessageEquals; import static org.junit.jupiter.api.AssertionTestUtils.assertMessageStartsWith; import static org.junit.jupiter.api.AssertionTestUtils.expectAssertionFailedError; -import static org.junit.jupiter.api.Assertions.assertLinesMatch; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; -import java.util.logging.LogRecord; +import java.util.Arrays; -import org.junit.jupiter.api.fixtures.TrackLogRecords; -import org.junit.platform.commons.logging.LogRecordListener; +import org.junit.platform.commons.JUnitException; import org.opentest4j.AssertionFailedError; /** @@ -632,14 +634,24 @@ void assertNotEqualsInvokesEqualsMethodForIdenticalObjects() { @Nested class AssertNotEqualsArrays { @Test - void objects(@TrackLogRecords LogRecordListener listener) { + void objects() { Object object = new Object(); Object array1 = new Object[] { object }; Object array2 = new Object[] { object }; - assertNotEquals(array1, array2); - assertLinesMatch(""" - Should have used `assertArrayEquals()` in method: - """.lines(), listener.stream(AssertionUtils.class).map(LogRecord::getMessage)); + assertThrows(AssertionFailedError.class, () -> assertSame(array1, array2)); + assertThrows(AssertionFailedError.class, () -> assertEquals(array1, array2)); + assertNotEquals(array1, array2); // succeeds + assertTrue(Arrays.deepEquals((Object[]) array1, (Object[]) array2)); + assertArrayEquals((Object[]) array1, (Object[]) array2); + try { + System.setProperty("junit.jupiter.disallow.arrays.in.equals.checks", "true"); + var exception = assertThrows(JUnitException.class, () -> assertNotEquals(array1, array2)); + assertEquals("Detected array arguments: class [Ljava.lang.Object; and class [Ljava.lang.Object;", + exception.getMessage()); + } + finally { + System.clearProperty("junit.jupiter.disallow.arrays.in.equals.checks"); + } } }