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