<i18n dev> RFR: JDK-8319569: Several java/util tests should be updated to accept VM flags. [v2]

[Justin Lu]

On Fri, 17 Nov 2023 19:36:21 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   reflect review comments
>
> test/jdk/java/util/Currency/PropertiesTest.sh line 30:
> 
>> 28: # @summary tests the capability of replacing the currency data with user
>> 29: #     specified currency properties file
>> 30: # @requires vm.flagless
> 
> Does this actually do anything? Since it is a shell script, it does not call any `ProcessBuilder` methods. In fact, the script includes `${TESTVMOPTS}` on launching the test java process correctly.

IIUC, using `@requires vm.flagless` simply signifies that any JVMs launched within the test do not support VM flags, (regardless if it was launched from ProcessBuilder).

However, it is odd that this test was flagged since it does pass `${TESTVMOPTS}` to `run()` like you stated. 

Perhaps @lmesnik has further understanding of what to do here?

> test/jdk/java/util/logging/LoggingDeadlock2.java line 167:
> 
>> 165: 
>> 166:    private static final List<String> javaChildArgs = Arrays.asList(
>> 167:        "-classpath", classpath, "LoggingDeadlock2$JavaChild");
> 
> Could use `List.of()`

Fixed here and in other instances.

> test/jdk/java/util/zip/EntryCount64k.java line 3:
> 
>> 1: /*
>> 2:  * Copyright (c) 2013 Google Inc. All rights reserved.
>> 3:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> 
> Is this OK? The header reads `DO NOT ALTER`.

Yes, I think you're right. I based my decision off https://github.com/openjdk/jdk/pull/15454

But after reading the copyright guidelines, we do not modify third party copyright. It should probably be removed in that test as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16705#discussion_r1397843189
PR Review Comment: https://git.openjdk.org/jdk/pull/16705#discussion_r1397843173
PR Review Comment: https://git.openjdk.org/jdk/pull/16705#discussion_r1397843159