On Tue, 18 May 2021 05:48:56 GMT, Alan Bateman <alanb@openjdk.org> wrote:
The changes look okay but a bit inconsistent on where -Djava...=allow is inserted for tests that already set other system properties or other parameters. Not a correctness issue, just looks odd in several places, e.g.
test/jdk/java/lang/System/LoggerFinder/BaseLoggerFinderTest/BaseLoggerFinderTest.java - the tests sets the system properties after -Xbootclasspath/a: but the change means it sets properties before and after.
test/jdk/java/lang/Class/getResource/ResourcesTest.java - you've added it in the middle of the module and class path parameters.
For uses using ProcessTools then it seems to be very random.
I've updated the 3 cases you mentioned and will go through more. Yes it looks good to group system property settings together. Thanks. ------------- PR: https://git.openjdk.java.net/jdk/pull/4071