On Mon, 18 May 2026 18:21:47 GMT, Chen Liang <[email protected]> wrote:
>> For preview of the serialization implementation cleanup. >> >> See the mainline diff version: >> https://gist.github.com/a3440f2b9da5111fe20315e59ed17380 >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Message updated test/jdk/java/io/Serializable/valueObjects/SimpleValueGraphs.java line 367: > 365: var obj = new ValueExt(); > 366: var ex = Assertions.assertThrows(InvalidClassException.class, () > -> serialize(obj)); > 367: Assertions.assertEquals("SimpleValueGraphs$ValueExt; cannot > serialize value class", ex.getMessage()); Shouldn't this error message say "externalize" rather than "serialize" (based off of the test name) test/jdk/java/io/Serializable/valueObjects/SimpleValueGraphs.java line 376: > 374: byte[] newBytes = patchBytes(bytes, "IdentExt", "ValueExt"); > 375: var ex = Assertions.assertThrows(InvalidClassException.class, () > -> deserialize(newBytes)); > 376: Assertions.assertTrue(ex.getMessage().contains("cannot serialize > value class")); same as https://github.com/openjdk/valhalla/pull/2326/changes#r3355827173 test/jdk/java/io/Serializable/valueObjects/ValueSerializationTest.java line 113: > 111: for (var f : > NonSerializableStrictPoint.class.getDeclaredFields()) { > 112: assertTrue(f.isStrictInit(), f.getName()); > 113: } I think it would be good to also validate that we are indeed seeing the fields (i.e. assert that we see "x" and "y") ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2326#discussion_r3355827173 PR Review Comment: https://git.openjdk.org/valhalla/pull/2326#discussion_r3355828400 PR Review Comment: https://git.openjdk.org/valhalla/pull/2326#discussion_r3355834591
