On Fri, 30 Oct 2026 08:14:45 GMT, Tobias Hartmann <[email protected]> wrote:
> When writing a test, I noticed that `assertNull(42)` fails with > `assertEquals expected: 42 but was: null` instead of `assertNull: expected > null but was 42`. So I thought "Let's quickly fix that!". Well, I found and > fixed more issues in the process and also added proper testing. > > Here's an overview: > | | | Before | After | > | --- | --- | --- | --- | > | **1** | `assertNull(42)` | `assertEquals expected: 42 but was: null` | > `assertNull: expected null but was 42` | > | **2** | `assertNotNull(null)` | `assertNotEquals: expected not equals but > was null` | `assertNotNull: expected non-null but was null` | > | **3** | `assertEqualsByteArray(null, new byte[1])` | `NullPointerException` > while formatting the failure | `assertEqualsByteArray: expected null but was > 00` | > | **4** | `assertLessThan(null, 42)` | `null: values must be non-null: <null> > , <42>` | `assertLessThan: values must be non-null: <null> , <42>` | > > I put the explanations in-line with the changes. > > Thanks, > Tobias > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). test/lib-test/jdk/test/lib/AssertsTest.java line 92: > 90: } > 91: > 92: public static void testErrorMessages() throws Exception { Added tests to check the error message for all methods that are already covered above in `main`. test/lib-test/jdk/test/lib/AssertsTest.java line 154: > 152: expectFail(Assertion.EQ, f1, f2); > 153: expectFail(Assertion.EQ, null, 2); > 154: expectFail(Assertion.EQ, 2, null); Fixed a copy-paste error. test/lib/jdk/test/lib/Asserts.java line 101: > 99: */ > 100: public static <T extends Comparable<T>>void assertLessThan(T lhs, T > rhs, String msg) { > 101: msg = Objects.toString(msg, "assertLessThan"); Fix for **(4)**: The `compare` below already detects `null` operands and will call `fail` with `msg` which is `null` at this point: https://github.com/openjdk/jdk/blob/d39a62afc66b5b15fb60bace0c7acee3f8169137/test/lib/jdk/test/lib/Asserts.java#L549-L551 We need to initialize `msg` before that. test/lib/jdk/test/lib/Asserts.java line 299: > 297: private static String formatBytes(byte[] bytes) { > 298: return bytes == null ? "null" : HexFormat.of().formatHex(bytes); > 299: } Fix for **(3)**: We need an explicit null check here to avoid an NPE during formatting: Exception in thread "main" java.lang.NullPointerException: Cannot read the array length because "bytes" is null at java.base/java.util.HexFormat.formatHex(HexFormat.java:320) at jdk.test.lib.Asserts.assertEqualsByteArray(Asserts.java:275) at jdk.test.lib.Asserts.assertEqualsByteArray(Asserts.java:249) at Test.main(Test.java:5) test/lib/jdk/test/lib/Asserts.java line 483: > 481: + ": expected null but was " + Objects.toString(o); > 482: fail(msg); > 483: } Fix for **(1)**: Original `assertEquals(o, null, msg);` check was the wrong way around. Also, it would print "assertEquals" which is misleading. test/lib/jdk/test/lib/Asserts.java line 508: > 506: + ": expected non-null but was null"; > 507: fail(msg); > 508: } Fix for **(2)**: Original `assertNotEquals(o, null, msg);` check was the wrong way around. Also, it would print "assertNotEquals" which is misleading. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421677644 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421672407 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421740077 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421709406 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421694596 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421715118
