On Tue, 14 Nov 2023 22:45:56 GMT, Mandy Chung <mch...@openjdk.org> wrote:
> This PR includes test fixes for the following issues:
>
> 8319567: Update java/lang/invoke tests to support vm flags
> 8319568: Update java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java
> to accept vm flags
> 8319672: Several classloader tests ignore VM flags
> 8319676: A couple of jdk/modules/incubator/ tests ignore VM flags
> 8319677: Test jdk/internal/misc/VM/RuntimeArguments.java should be marked as
> flagless
>
> It converts the test to use `ProcessTools::createTestJavaProcessBuilder` or
> `createNativeTestJavaProcessBuilder` so that the test will support VM flags
> passed to jtreg. A couple tests that ignore VM flags should use
> `ProcessTools::createLimtiedTestJavaProcessBuilder` and marks the test with
> `@requires vm.flagless`.
test/jdk/java/lang/ClassLoader/getResource/GetResource.java line 166:
> 164: Map<String,String> env = pb.environment();
> 165: String value = env.remove("CLASSPATH");
> 166:
Looking into the implementation, it seems that the `CLASSPATH` environment
variable is only cleared when `test.noclasspath` == true:
if (noCP) {
// clear CLASSPATH from the env
pb.environment().remove("CLASSPATH");
}
(This seems to be contrary to the doc comment on `createTestJavaProcessBuilder`
though, which says that _unless_ `test.noclasspath` is true, the env. var will
be cleared).
Should this test be run with `-Dtest.noclasspath=true`?
test/jdk/java/lang/invoke/findSpecial/FindSpecialTest.java line 60:
> 58: }
> 59: String classpath = m1.toString() + File.pathSeparator +
> TEST_CLASS_PATH;
> 60: executeCommand(createTestJavaProcessBuilder( "-cp", classpath,
Suggestion:
executeCommand(createTestJavaProcessBuilder("-cp", classpath,
test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java line 236:
> 234: ProcessBuilder pb = createLimitedTestJavaProcessBuilder(
> 235: "-cp", CLASSES.toString(),
> 236: "-Djava.security.manager=allow",
Suggestion:
"-cp", CLASSES.toString(),
"-Djava.security.manager=allow",
test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java line 241:
> 239: executeProcess(pb)
> 240: .shouldContain("DUMP_LAMBDA_PROXY_CLASS_FILES is not
> writable")
> 241: .shouldNotHaveExitValue(0);
The old code also seems to test that the error is shown exactly once (and not
multiple times). Not sure if that is important to retain?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16666#discussion_r1393471863
PR Review Comment: https://git.openjdk.org/jdk/pull/16666#discussion_r1393473298
PR Review Comment: https://git.openjdk.org/jdk/pull/16666#discussion_r1393479834
PR Review Comment: https://git.openjdk.org/jdk/pull/16666#discussion_r1393480987
