On Thu, 4 Dec 2025 22:38:39 GMT, Roger Riggs <[email protected]> wrote:
>> This test failing with VM options prompted a refatoring and cleanup of the
>> test (and renaming to be more appropriate).
>> The test is re-enabled with a 4 combinations of command line flags.
>>
>> Refactored test to verify that hashcode changed with each change to a field.
>> Previously, the test tried to compute the hashCode for the value class. That
>> was fragile in the presence of the VM changing the layout information. Check
>> the hashCodes are equal for objects that are `.equals`. Renamed to
>> 'ValueObjectMethodsTest`.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Correct check for identityHashCode
test/jdk/valhalla/valuetypes/ValueObjectMethodsTest.java line 253:
> 251: Line l3 = new Line(0, 9, 2, 3);
> 252: Line l4 = new Line(0, 1, 9, 3);
> 253: Line l5 = new Line(0, 1, 2, 9);
Random thought: Would it be a good idea to randomize the content to increase
coverage over time? For example,
var r = new java.util.Random();
var coordinates = Stream
.generate(() -> List.of(r.nextInt(), r.nextInt()))
.distinct()
.limit(4)
.toList();
int i = 0;
var p1 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1));
i++;
var p2 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1));
i++;
var p3 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1));
i++;
var p4 = new Point(coordinates.get(i).get(0), coordinates.get(i).get(1));
test/jdk/valhalla/valuetypes/ValueObjectMethodsTest.java line 280:
> 278: @MethodSource("hashcodeTests")
> 279: public void testHashCode(List<Object> objects) {
> 280: assertTrue(objects.size() > 1, "More than one object is
> required: " + objects);
_Nit:_ I doubt if displaying `[]` will contribute much to the diagnostics
context.
Suggestion:
assertFalse(objects.isEmpty(), "More than one object is required");
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1771#discussion_r2591865591
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1771#discussion_r2591872272
