On Thu, 18 Dec 2025 23:01:07 GMT, Justin Lu <[email protected]> wrote:

> Please review this PR which migrates the java.time tests from TestNG to 
> JUnit. The java.time tests use TestNG based on the directory level settings 
> configured by TEST.properties, so they are best migrated altogether. This is 
> a large PR, so I have tried to make the changes clear by commit.
> 
> First, the auto conversion tool is run in 
> https://github.com/openjdk/jdk/commit/b1fd7dbdec85aac5a44cc875e57a36be8f1b6974.
> https://github.com/openjdk/jdk/commit/3805cfd8765c0c76b61893dcf1670951402f98c3
>  and 
> https://github.com/openjdk/jdk/commit/b697ca5d9a8067bcecea2dfb32f92f7699085dee
>  are required so that the tests can actually compile and run.
> https://github.com/openjdk/jdk/commit/d07c912c4c16d2b3307e489563f148f71cfdf4a4
>  addresses the timeout annotation which was not covered by the auto 
> conversion tool.
> The rest of the commits are aesthetic related.
> 
> Before conversion stats
> 
> 
> Test results: passed: 187
> Framework-based tests: 32,339 = 32,339 TestNG + 0 JUnit
> 
> 
> After conversion stats
> 
> 
> Test results: passed: 187
> Framework-based tests: 32,339 = 0 TestNG + 32,339 JUnit

LGTM over all. There may be a bug in the tool, as I see duplicated comments for 
no apparent reason (did not check all the files)
Also, please rename the directory `java/time/nontestng` to `java/time/nonjunit`.

test/jdk/java/time/tck/java/time/chrono/serial/TCKChronoLocalDateSerialization.java
 line 89:

> 87:     static final int HIJRAH_DATE_TYPE = 6;       // 
> java.time.chrono.Ser.HIJRAH_DATE_TYPE
> 88:     static final int MINGUO_DATE_TYPE = 7;       // 
> java.time.chrono.Ser.MINGUO_DATE_TYPE
> 89:     static final int THAIBUDDHIST_DATE_TYPE = 8; // 
> java.time.chrono.Ser.THAIBUDDHIST_DATE_TYPE// 
> java.time.chrono.Ser.THAIBUDDHIST_DATE_TYPE

The comment seems duplicated for no apparent reason. (Tool's bug?)

test/jdk/java/time/tck/java/time/chrono/serial/TCKChronologySerialization.java 
line 76:

> 74: public class TCKChronologySerialization extends AbstractTCKTest {
> 75: 
> 76:     static final int CHRONO_TYPE = 1;            // 
> java.time.chrono.Ser.CHRONO_TYPE// java.time.chrono.Ser.CHRONO_TYPE

Same here

test/jdk/java/time/tck/java/time/chrono/serial/TCKEraSerialization.java line 95:

> 93: public class TCKEraSerialization extends AbstractTCKTest {
> 94: 
> 95:     static final int JAPANESE_ERA_TYPE = 5;     // 
> java.time.chrono.Ser.JAPANESE_ERA// java.time.chrono.Ser.JAPANESE_ERA

And here too

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

PR Review: https://git.openjdk.org/jdk/pull/28911#pullrequestreview-3599617701
PR Review Comment: https://git.openjdk.org/jdk/pull/28911#discussion_r2635940661
PR Review Comment: https://git.openjdk.org/jdk/pull/28911#discussion_r2635942887
PR Review Comment: https://git.openjdk.org/jdk/pull/28911#discussion_r2635944980

Reply via email to