On Fri, 10 Jul 2026 20:35:35 GMT, Justin Lu <[email protected]> wrote:

>> Sholto has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   8272194: introduce faster code path for LocalDate/LocalDateTime conversion
>
> test/jdk/java/sql/test/sql/DateTests.java line 1:
> 
>> 1: /*
> 
> Since there is good traction on this PR, I think you can go ahead and update 
> the copyright years for any files that need it. Also, since these tests don't 
> have individual Jtreg headers, I think it is worth including the JBS bug ID 
> 8272194 in the test comments to better associate the change with the test.

Done as suggested.

> test/jdk/java/sql/test/sql/DateTests.java line 338:
> 
>> 336:         Date d1 = Date.valueOf(ld1);
>> 337:         LocalDate ld2 = d1.toLocalDate();
>> 338:         assertTrue(ld1.equals(ld2), "Error ld1 != ld2");
> 
> You might use `assertEquals` here and in the other occurrences as a more 
> accurate assertion. Perhaps also update the error message to make it clear 
> that the failed assertion is about negative years.

Done. I have also improved the test adding more coverage and improving the 
error messages.
I have also moved away from the negative year nomenclature as that obfuscates 
year 0 (1BC).
I have added tests to ensure we are checking this year 0 case as well, as it is 
an obvious possible boundary condition.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3570071083
PR Review Comment: https://git.openjdk.org/jdk/pull/31808#discussion_r3570080502

Reply via email to