On Thu, 22 Feb 2024 16:01:19 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> Currently assertEquals has in the failure case sometimes confusing output >> like : >> >> java.lang.RuntimeException: VM output should contain exactly one RTM locking >> statistics entry for method >> compiler.rtm.locking.TestRTMTotalCountIncrRate$Test:🔒 expected 0 to equal 1 >> at jdk.test.lib.Asserts.fail(Asserts.java:634) >> at jdk.test.lib.Asserts.assertEquals(Asserts.java:205) >> >> (I don't think we really expected that for some reason 0 equals 1) >> This should be improved. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > assertEquals adjust output Hello Matthias, the proposal to improve that failure message looks OK to me. However, I feel that the newly proposed error message isn't too different than what it was earlier. Perhaps, we could do something like: diff --git a/test/lib/jdk/test/lib/Asserts.java b/test/lib/jdk/test/lib/Asserts.java index d503ea8e544..3b45d7f4129 100644 --- a/test/lib/jdk/test/lib/Asserts.java +++ b/test/lib/jdk/test/lib/Asserts.java @@ -199,10 +199,11 @@ public static void assertEquals(Object lhs, Object rhs) { */ public static void assertEquals(Object lhs, Object rhs, String msg) { if ((lhs != rhs) && ((lhs == null) || !(lhs.equals(rhs)))) { - msg = Objects.toString(msg, "assertEquals") - + ": expected " + Objects.toString(lhs) - + " to equal " + Objects.toString(rhs); - fail(msg); + if (msg != null) { + fail(msg); + } else { + fail("assertEquals expected: " + lhs + " but was: " + rhs); + } } } This is similar to what other test libraries usually report for such failures. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17952#issuecomment-1962337015