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). I like that improvement! I have a weird interest for error messages, reporting, tracking locus and extents... So, I have a few comments, that you can feel free to ignore. test/lib-test/jdk/test/lib/AssertsTest.java line 103: > 101: > 102: byte[] zero = new byte[1]; > 103: expectFailure("assertEqualsByteArray: expected 00 but was 0000", > () -> Asserts.assertEqualsByteArray(zero, new byte[2])); I bet this printout is going to be confusing. I shortly was, and probably less shortly if I couldn't see immediately the definition of both operands. But it's compact. Maybe good enough! test/lib-test/jdk/test/lib/AssertsTest.java line 106: > 104: expectFailure("assertEqualsByteArray: expected null but was 00", > () -> Asserts.assertEqualsByteArray(null, zero)); > 105: expectFailure("assertEqualsByteArray: expected 00 but was null", > () -> Asserts.assertEqualsByteArray(zero, null)); > 106: expectFailure("assertNotEqualsByteArray: expected not equals but > was 00", () -> Asserts.assertNotEqualsByteArray(zero, zero)); Again, I'm not fan of this phrasing, but it was like that before. I don't love that it's singular, but applies to both operands (implicitly). Again, maybe good enough. test/lib-test/jdk/test/lib/AssertsTest.java line 123: > 121: > 122: expectFailure("assertTrue: expected true, was false", () -> > Asserts.assertTrue(false)); > 123: expectFailure("assertFalse: expected false, was true", () -> > Asserts.assertFalse(true)); Isn't it a bit redundant? "assertFalse, expected false"? But we lack macros to show which property is wrongly true. I find that awkward, but no better idea. ------------- Marked as reviewed by mchevalier (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/33643#pullrequestreview-5709920724 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421849183 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421860904 PR Review Comment: https://git.openjdk.org/jdk/pull/33643#discussion_r4421866910
