On Tue, 29 Aug 2023 09:12:51 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. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > copyright > I kind of like that it is long and clumsy, that makes it harder to use... ...but it's used in 462 files. That implies it is commonly used. Are you suggesting nearly all those uses are incorrect and eventually should be converted to `createTestJvm()`? ------------- PR Comment: https://git.openjdk.org/jdk/pull/15452#issuecomment-1698003837
