On Wed, 25 Oct 2023 08:44:29 GMT, Leo Korinth <lkori...@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