On Tue, 28 Mar 2023 04:46:52 GMT, Mandy Chung <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - don't iterate over the properties file keys and instead do lookup when
>> needed
>> - update CDSPluginTest to correctly "simulate" cross-platform test
>
> test/jdk/tools/jlink/plugins/CDSPluginTest.java line 98:
>
>> 96: // the java.base module-info.class to identify the target
>> platform for the image
>> 97: // being generated.
>> 98: Path jdkRoot = Path.of(jlinkPath).getParent().getParent();
>
> Or simply get from `test.jdk` system property:
>
> Suggestion:
>
> Path jdkRoot = Path.of(System.getProperty("test.jdk"));
Hello Mandy, the `JDKToolFinder.getJDKTool("jlink")` uses the `test.jdk` to try
and find the tool and then falls back to another system property, if the tool
wasn't found. That's why I had used this `getParent()` technique to locate the
JDK root. But I think using just the `test.jdk` system property should be fine
too, since as far as I can see in the test definition of this test, there isn't
anything specific that would require the test JDK to be present in any
different place than the `test.jdk` system property. I've updated the PR to use
this suggested approach.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1150081505
