On Tue, 17 May 2022 20:19:28 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> Gaurav Chaudhari has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8285838: Revised changes for Custom TZ DST test fixes.
>
> src/java.base/unix/native/libjava/TimeZone_md.c line 609:
> 
>> 607:     }
>> 608: 
>> 609:     offset = (gmt.tm_hour - localtm.tm_hour)*3600 + (gmt.tm_min - 
>> localtm.tm_min)*60;
> 
> Since it is not using `timezone` anymore, we can reverse the subtraction, 
> i.e., `localtime` - `gmtime` so that the weird sign switch below can be 
> eliminated.

I've reversed it and reverted the weird sign switching below this code snippet.

> test/jdk/java/util/TimeZone/CustomTzIDCheckDST.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> This is a new test case, the year should be only 2022.

Thanks, corrected this in the following commit

> test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 1:
> 
>> 1: #
> 
> I'd change this script into a java test case (using `.sh` is not 
> recommended). In fact, this causes a failure invoking `javac` with `-Xmx768m` 
> (from TESTVMOPTS)
> There are libraries under `jdk/test/lib/` for building test jvm process and 
> tools.

I did away with this .sh script and combined the shell script and java test 
into one. Initially the only purpose of this shell script was in order to set 
up the environment variable for the proceeding test. However, I learnt that all 
this could be done within java, and I consolidated both the files into one test 
now.

> test/jdk/java/util/TimeZone/runCustomTzIDCheckDST.sh line 40:
> 
>> 38: 
>> 39: OS=`uname -s`
>> 40: case "$OS" in 
> 
> In case other than Linux/AIX, it would be helpful to emit some message that 
> the test is ignored.

All moved to one java test now.

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

PR: https://git.openjdk.java.net/jdk/pull/8661

Reply via email to