<i18n dev> RFR: 8315097: Rename createJavaProcessBuilder [v7]

[Roger Riggs]

On Wed, 25 Oct 2023 08:44:29 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> This pull request renames `createJavaProcessBuilder` to `createLimitedTestJavaProcessBuilder` and renames `createTestJvm` to `createTestJavaProcessBuilder`. Both are implemented through a private `createJavaProcessBuilder`. It also updates the java doc.
>> 
>> This is so that it should be harder to by mistake use `createLimitedTestJavaProcessBuilder` that is problematic because it will not forward JVM flags to the tested JVM.
>
> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix copyright year and indentation

Suggestions to complete the descriptions of the createXXXJavaProcessBuilder methods.

test/lib/jdk/test/lib/process/ProcessTools.java line 505:

> 503:      * @return The ProcessBuilder instance representing the java command.
> 504:      */
> 505:     public static ProcessBuilder createTestJavaProcessBuilder(List<String> command) {

Include the same description of other properties that are included in creating the ProcessBuilder...
```     * Unless the "test.noclasspath" property is "true"
     * the classpath property "java.class.path" is appended to the command line and
     * the environment of the ProcessBuilder is modified to remove "CLASSPATH".
     * If the property "test.thread.factory" is provided the command args are
     * updated and appended to invoke ProcessTools main() and provide the 
     * name of the thread factory.
     * The "-Dtest.thread.factory" is appended to the arguments with the thread factory value.
     * The remaining command args are scanned for unsupported options and 
     * are appended to the ProcessBuilder.

test/lib/jdk/test/lib/process/ProcessTools.java line 520:

> 518:      * @return The ProcessBuilder instance representing the java command.
> 519:      */
> 520:     public static ProcessBuilder createTestJavaProcessBuilder(String... command) {

Include the same description of other properties that are included in creating the ProcessBuilder...

     * Unless the "test.noclasspath" property is "true"
     * the classpath property "java.class.path" is appended to the command line and
     * the environment of the ProcessBuilder is modified to remove "CLASSPATH".
     * If the property "test.thread.factory" is provided the command args are
     * updated and appended to invoke ProcessTools main() and provide the 
     * name of the thread factory.
     * The "-Dtest.thread.factory" is appended to the arguments with the thread factory value.
     * The remaining command args are scanned for unsupported options and 
     * are appended to the ProcessBuilder.

test/lib/jdk/test/lib/process/ProcessTools.java line 538:

> 536:      * it in combination with <b>@requires vm.flagless</b> JTREG
> 537:      * anotation as to not waste energy and test resources.
> 538:      *

Consider adding this description of what this method does.
Suggestion:

     * Unless the "test.noclasspath" property is "true"
     * the classpath property "java.class.path" is appended to the command line and
     * the environment of the ProcessBuilder is modified to remove "CLASSPATH".
     * If the property "test.thread.factory" is provided the command args are
     * updated and appended to invoke ProcessTools main() and provide the 
     * name of the thread factory.
     * The "-Dtest.thread.factory" is appended to the arguments with the thread factory value.
     * The remaining command args are scanned for unsupported options and 
     * are appended to the ProcessBuilder.

test/lib/jdk/test/lib/process/ProcessTools.java line 560:

> 558:      * it in combination with <b>@requires vm.flagless</b> JTREG
> 559:      * anotation as to not waste energy and test resources.
> 560:      *

Suggestion:

     * Unless the "test.noclasspath" property is "true"
     * the classpath property "java.class.path" is appended to the command line and
     * the environment of the ProcessBuilder is modified to remove "CLASSPATH".
     * If the property "test.thread.factory" is provided the command args are
     * updated and appended to invoke ProcessTools main() and provide the 
     * name of the thread factory.
     * The "-Dtest.thread.factory" is appended to the arguments with the thread factory value.
     * The remaining command args are scanned for unsupported options and 
     * are appended to the ProcessBuilder.

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

PR Review: https://git.openjdk.org/jdk/pull/15452#pullrequestreview-1698308785
PR Review Comment: https://git.openjdk.org/jdk/pull/15452#discussion_r1372364800
PR Review Comment: https://git.openjdk.org/jdk/pull/15452#discussion_r1372364171
PR Review Comment: https://git.openjdk.org/jdk/pull/15452#discussion_r1372361862
PR Review Comment: https://git.openjdk.org/jdk/pull/15452#discussion_r1372362333