On Mon, 3 Jun 2024 20:24:49 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Alex's input - simplify the test by using ClassFileInstaller
>
> test/jdk/java/lang/instrument/NativeMethodPrefixApp.java line 86:
>
>> 84: Can-Set-Native-Method-Prefix: true
>> 85: """
>> 86: + "Boot-Class-Path: " +
>> bootClassesDir.toString().replace(File.separatorChar, '/') + "/"
>
> AFAIU Boot-Class-Path is needed only because the agent needs
> bootreporter.StringIdCallbackReporter
> I don't see much point in separate bootreporter dir.
> So we can set Boot-Class-Path to System.getProperty("test.classes") and drop
> createBootClassesDir() logic
Hello Alex, I have updated the PR to add "test.classes" to Boot-Class-Path
instead of just those 2 classes. Tests in the updated PR continue to pass.
> test/jdk/java/lang/instrument/RetransformApp.java line 81:
>
>> 79: final OutputAnalyzer oa = ProcessTools.executeTestJava(
>> 80: "--enable-preview", // due to usage of ClassFile API
>> PreviewFeature in the agent
>> 81: "-XX:+UnlockDiagnosticVMOptions", "-XX:-CheckIntrinsics",
>
> This `-XX` flags were not present for this test (and I don't think they are
> needed for NativeMethodPrefix test too)
You are right - it was a copy/paste error on my part to have include these
flags for the `RetransformApp`. Based on your suggestion, I have removed them
from the other test too. The tests continue to pass.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19495#discussion_r1625219101
PR Review Comment: https://git.openjdk.org/jdk/pull/19495#discussion_r1625219542
