From 35e86b20436e46d3f75cf1624fe7ca173386cf48 Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Tue, 2 Sep 2025 11:31:28 +0200 Subject: [PATCH 1/3] 7904072: Fail for arrays in JUnit assert[Not]Equals methods This commit introduces a patch for JUnit's AssertionUtils class to throw an exception when arrays are used in assert[Not]Equals methods --- make/jtreg.gmk | 6 +- .../regtest/exec/RegressionScript.java | 10 +- .../org/junit/jupiter/api/AssertionUtils.java | 124 ++++++++++++++++++ test/junitTrace/JUnitTrace.gmk | 3 +- .../JupiterAssertEqualsWithArraysTests.java | 45 +++++++ 5 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 src/share/classes/org/junit/jupiter/api/AssertionUtils.java create mode 100644 test/junitTrace/JupiterAssertEqualsWithArraysTests.java diff --git a/make/jtreg.gmk b/make/jtreg.gmk index 136e0915c..beb863ddd 100644 --- a/make/jtreg.gmk +++ b/make/jtreg.gmk @@ -64,7 +64,8 @@ $(BUILDDIR)/classes.com.sun.javatest.regtest.ok: \ -d $(CLASSDIR) \ -encoding ASCII \ $(TOOL_DEBUG_FLAGS) \ - $(JAVAFILES.com.sun.javatest.regtest-tools) + $(JAVAFILES.com.sun.javatest.regtest-tools) \ + $(JAVADIR)/org/junit/jupiter/api/AssertionUtils.java echo "classes built at `date`" > $@ TARGETS.com.sun.javatest.regtest += $(BUILDDIR)/classes.com.sun.javatest.regtest.ok @@ -237,7 +238,8 @@ PKGS.JAR.jtreg += \ com.sun.javatest.regtest.report \ com.sun.javatest.regtest.tool \ com.sun.javatest.regtest.util \ - java.lang + java.lang \ + org.junit.jupiter.api TARGETS.JAR.jtreg += $(TARGETS.com.sun.javatest.regtest) FILES.JAR.jtreg=$(CLASSDIR)/META-INF/services/java.util.spi.ToolProvider diff --git a/src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java b/src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java index b65e5ae2b..2af1c8ef2 100644 --- a/src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java +++ b/src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java @@ -937,6 +937,11 @@ Map getExecutionPaths( pp.append(locations.absLibClsList(LibLocn.Kind.SYS_MODULE)); } + // javatest.jar and jtreg.jar + if (include_jtreg) { + (testOnBootClassPath ? bcp : cp).append(getJavaTestClassPath()); + } + // Frameworks: if (multiModule) { // assert !testOnBootClassPath && !useXPatch() @@ -972,11 +977,6 @@ Map getExecutionPaths( cp.append(cpa); } - // javatest.jar and jtreg.jar - if (include_jtreg) { - (testOnBootClassPath ? bcp : cp).append(getJavaTestClassPath()); - } - Map map = new EnumMap<>(PathKind.class); if (!bcp.isEmpty()) map.put(PathKind.BOOTCLASSPATH_APPEND, bcp); diff --git a/src/share/classes/org/junit/jupiter/api/AssertionUtils.java b/src/share/classes/org/junit/jupiter/api/AssertionUtils.java new file mode 100644 index 000000000..ef05db02b --- /dev/null +++ b/src/share/classes/org/junit/jupiter/api/AssertionUtils.java @@ -0,0 +1,124 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +/* + * Based on https://github.com/junit-team/junit-framework/commits/main/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertionUtils.java + */ + +package org.junit.jupiter.api; + +import static java.util.stream.Collectors.joining; + +import java.util.Deque; +import java.util.function.Supplier; + +import org.junit.platform.commons.util.UnrecoverableExceptions; +import org.opentest4j.AssertionFailedError; + +/** + * {@code AssertionUtils} is a collection of utility methods that are common to + * all assertion implementations. + * + * @since 5.0 + */ +class AssertionUtils { + + private AssertionUtils() { + /* no-op */ + } + + static void fail() { + throw new AssertionFailedError(); + } + + static void fail(String message) { + throw new AssertionFailedError(message); + } + + static void fail(String message, Throwable cause) { + throw new AssertionFailedError(message, cause); + } + + static void fail(Throwable cause) { + throw new AssertionFailedError(null, cause); + } + + static void fail(Supplier messageSupplier) { + throw new AssertionFailedError(nullSafeGet(messageSupplier)); + } + + static String nullSafeGet(Supplier messageSupplier) { + return (messageSupplier != null ? messageSupplier.get() : null); + } + + static String getCanonicalName(Class clazz) { + try { + String canonicalName = clazz.getCanonicalName(); + return (canonicalName != null ? canonicalName : clazz.getTypeName()); + } + catch (Throwable t) { + UnrecoverableExceptions.rethrowIfUnrecoverable(t); + return clazz.getTypeName(); + } + } + + static String formatIndexes(Deque indexes) { + if (indexes == null || indexes.isEmpty()) { + return ""; + } + String indexesString = indexes.stream().map(Object::toString).collect(joining("][", "[", "]")); + return " at index " + indexesString; + } + + static boolean floatsAreEqual(float value1, float value2, float delta) { + assertValidDelta(delta); + return floatsAreEqual(value1, value2) || Math.abs(value1 - value2) <= delta; + } + + static void assertValidDelta(float delta) { + if (Float.isNaN(delta) || delta < 0.0) { + failIllegalDelta(String.valueOf(delta)); + } + } + + static void assertValidDelta(double delta) { + if (Double.isNaN(delta) || delta < 0.0) { + failIllegalDelta(String.valueOf(delta)); + } + } + + static boolean floatsAreEqual(float value1, float value2) { + return Float.floatToIntBits(value1) == Float.floatToIntBits(value2); + } + + static boolean doublesAreEqual(double value1, double value2, double delta) { + assertValidDelta(delta); + return doublesAreEqual(value1, value2) || Math.abs(value1 - value2) <= delta; + } + + static boolean doublesAreEqual(double value1, double value2) { + return Double.doubleToLongBits(value1) == Double.doubleToLongBits(value2); + } + + static boolean objectsAreEqual(Object obj1, Object obj2) { + if (obj1 == null) { + return (obj2 == null); + } + if (obj1.getClass().isArray() && (obj2 != null && obj2.getClass().isArray())) { + throw new AssertionError("Should have used `assertArrayEquals()`?!"); + } + return obj1.equals(obj2); + } + + private static void failIllegalDelta(String delta) { + fail("positive delta expected but was: <" + delta + ">"); + } + +} diff --git a/test/junitTrace/JUnitTrace.gmk b/test/junitTrace/JUnitTrace.gmk index 6e6690ed4..ee115d2cb 100644 --- a/test/junitTrace/JUnitTrace.gmk +++ b/test/junitTrace/JUnitTrace.gmk @@ -37,10 +37,11 @@ $(BUILDTESTDIR)/JUnitTrace.othervm.ok: \ $(TESTDIR)/junitTrace/ \ > $(@:%.ok=%/jt.log) 2>&1 || \ true "non-zero exit code from JavaTest intentionally ignored" - $(GREP) -s 'Test results: passed: 3; failed: 2' $(@:%.ok=%/jt.log) > /dev/null + $(GREP) -s 'Test results: passed: 3; failed: 3' $(@:%.ok=%/jt.log) > /dev/null $(GREP) -s "java.lang.NullPointerException: NPE" $(@:%.ok=%/work/NPE.jtr) > /dev/null $(GREP) -s "Intentionally thrown before all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null $(GREP) -s "Intentionally thrown after all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null + $(GREP) -s "Should have used `assertArrayEquals()`" $(@:%.ok=%/work/JupiterAssertEqualsWithArraysTests.jtr) > /dev/null if $(GREP) -s "^\s\s*at " $(@:%.ok=%/work/Pass.jtr) > /dev/null ; then \ echo "unexpected text"; exit 1; \ fi diff --git a/test/junitTrace/JupiterAssertEqualsWithArraysTests.java b/test/junitTrace/JupiterAssertEqualsWithArraysTests.java new file mode 100644 index 000000000..53cdd6e62 --- /dev/null +++ b/test/junitTrace/JupiterAssertEqualsWithArraysTests.java @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import org.junit.jupiter.api.*; + +/* + * @test + * @run junit JupiterAssertEqualsWithArraysTests + */ +class JupiterAssertEqualsWithArraysTests { + @Test + void arraysInAssertEqualsShouldThrow() { + Object o = new Object(); + Object a1 = new Object[] {o}; + Object a2 = new Object[] {o}; + Assertions.assertEquals(a1, a2); + } + + @Test + void arraysInAssertNotEqualsShouldThrow() { + Object a1 = new int[] {99}; + Object a2 = new int[] {99}; + Assertions.assertNotEquals(a1, a2); + } +} From a51bf52d69ceff6f8608e3629b8fdcbd7338a905 Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Tue, 2 Sep 2025 11:39:28 +0200 Subject: [PATCH 2/3] Fix bash/grep syntax --- test/junitTrace/JUnitTrace.gmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/junitTrace/JUnitTrace.gmk b/test/junitTrace/JUnitTrace.gmk index ee115d2cb..5c5ad72aa 100644 --- a/test/junitTrace/JUnitTrace.gmk +++ b/test/junitTrace/JUnitTrace.gmk @@ -41,7 +41,7 @@ $(BUILDTESTDIR)/JUnitTrace.othervm.ok: \ $(GREP) -s "java.lang.NullPointerException: NPE" $(@:%.ok=%/work/NPE.jtr) > /dev/null $(GREP) -s "Intentionally thrown before all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null $(GREP) -s "Intentionally thrown after all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null - $(GREP) -s "Should have used `assertArrayEquals()`" $(@:%.ok=%/work/JupiterAssertEqualsWithArraysTests.jtr) > /dev/null + $(GREP) -s "Should have used \\\`assertArrayEquals()\\\`" $(@:%.ok=%/work/JupiterAssertEqualsWithArraysTests.jtr) > /dev/null if $(GREP) -s "^\s\s*at " $(@:%.ok=%/work/Pass.jtr) > /dev/null ; then \ echo "unexpected text"; exit 1; \ fi From a8276a410a509981abdd10a99aafe26a107f1687 Mon Sep 17 00:00:00 2001 From: Christian Stein Date: Tue, 2 Sep 2025 11:44:45 +0200 Subject: [PATCH 3/3] Ignore backtick characters --- test/junitTrace/JUnitTrace.gmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/junitTrace/JUnitTrace.gmk b/test/junitTrace/JUnitTrace.gmk index 5c5ad72aa..ef0fca067 100644 --- a/test/junitTrace/JUnitTrace.gmk +++ b/test/junitTrace/JUnitTrace.gmk @@ -41,7 +41,7 @@ $(BUILDTESTDIR)/JUnitTrace.othervm.ok: \ $(GREP) -s "java.lang.NullPointerException: NPE" $(@:%.ok=%/work/NPE.jtr) > /dev/null $(GREP) -s "Intentionally thrown before all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null $(GREP) -s "Intentionally thrown after all" $(@:%.ok=%/work/JupiterLifecycle.jtr) > /dev/null - $(GREP) -s "Should have used \\\`assertArrayEquals()\\\`" $(@:%.ok=%/work/JupiterAssertEqualsWithArraysTests.jtr) > /dev/null + $(GREP) -s "Should have used .assertArrayEquals()." $(@:%.ok=%/work/JupiterAssertEqualsWithArraysTests.jtr) > /dev/null if $(GREP) -s "^\s\s*at " $(@:%.ok=%/work/Pass.jtr) > /dev/null ; then \ echo "unexpected text"; exit 1; \ fi