On Mon, 28 Aug 2023 15:54:08 GMT, Leo Korinth <lkori...@openjdk.org> wrote:

> Rename createJavaProcessBuilder so that it is not used by mistake instead of 
> createTestJvm.
> 
> I have used the following sed script: `find -name "*.java" | xargs -n 1 sed 
> -i -e 
> "s/createJavaProcessBuilder(/createJavaProcessBuilderIgnoreTestJavaOpts(/g"`
> 
> Then I have manually modified ProcessTools.java. In that file I have moved 
> one version of createJavaProcessBuilder so that it is close to the other 
> version. Then I have added a javadoc comment in bold telling:
> 
>    /**
>      * Create ProcessBuilder using the java launcher from the jdk to
>      * be tested.
>      *
>      * <p><b> Please observe that you likely should use
>      * createTestJvm() instead of this method because createTestJvm()
>      * will add JVM options from "test.vm.opts" and "test.java.opts"
>      * </b> and this method will not do that.
>      *
>      * @param command Arguments to pass to the java command.
>      * @return The ProcessBuilder instance representing the java command.
>      */
> 
> 
> I have used the name createJavaProcessBuilderIgnoreTestJavaOpts because of 
> the name of Utils.prependTestJavaOpts that adds those VM flags. If you have a 
> better name I could do a rename of the method. I kind of like that it is long 
> and clumsy, that makes it harder to use...
> 
> I have run tier 1 testing, and I have started more exhaustive testing.

The. changes looks good. Please update copyrights for changed filed.
I expect that you completed execution of all tests to ensure that nothing is 
broken.

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

Marked as reviewed by lmesnik (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15452#pullrequestreview-1598884378

Reply via email to