On Fri, 31 May 2024 12:01:14 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review of this test-only change which addresses >> https://bugs.openjdk.org/browse/JDK-8333130? >> >> There are a couple of tests `NativeMethodPrefixApp` and `RetransformApp` >> under `test/jdk/java/lang/instrument/` which launch the app/test with a >> `-javaagent:<jar>` pointing to a test specific jar. The test, launched with >> that `javaagent`, then verifies the test specific details. >> >> In their current form, in order to construct the agent `.jar` and make it >> available to the jtreg test that's launched, they `@run` a jtreg `shell` >> test. This `shell` test uses the `MakeJAR2.sh` script to generate the jar >> file. The contents of the script is relatively straightforward - it compiles >> (using `javac`) some boot classpath classes, some agent specific classes and >> then generates a jar file with the agent classes and a `MANIFEST.MF` which >> points to the boot classpath classes along with test specific manifest >> attributes. In a recent PR the `MakeJAR2.sh` script was updated to pass >> `--enable-preview --release 23` since one of the existing agent class was >> updated to use the ClassFile API PreviewFeature >> (https://github.com/openjdk/jdk/pull/13009). >> >> This generated agent jar then is passed as `-javaagent:` to the subsequent >> `@run main` test which does the actual testing. >> >> So essentially the `shell` test which uses the `MakeJAR2.sh` is merely there >> to create a jar file that's needed by the actual test. Within the JDK we >> have several tests which compile other classes and generate jar files using >> in-process test libraries and the `java.util.spi.ToolProvider` >> implementations. These 2 tests too can be updated to use a similar >> technique, to completely get rid of the `MakeJAR2.sh`. Removing the >> `MakeJAR2.sh` will simplify the ability to pass around value for `--release` >> when using `--enable-preview`. >> >> The commit in this PR updates these 2 tests to use `@run driver` which then >> compiles relevant classes (maintaining the semantics of `MakeJAR2.sh`) and >> generates the agent jar file and then finally launching the test process. As >> part of the update, I did not retain the `@author` tag from the 2 test >> definitions, since it's my understanding that we no longer use those. I will >> add them back if we should retain them. >> >> The tests continue to pass locally with this change. I've also triggered >> tier testing in our CI for this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > fix Boot-Class-Path value for Windows Hello Alex, what you suggest seems interesting and like you note much more simpler. I'll try it out and if that will work fine with the use of `--release` for the `asmlib/Instrumentor.java`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/19495#issuecomment-2143194801
