On Sat, 23 Nov 2024 10:32:28 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> Can I get a review of this test-only cleanup/speedup PR which modernizes the
>> test `RemoveJar.java`.
>>
>> This test attempts a variety of class loading scenarios on a URLClassLoader
>> and verifies that when the loader is closed, it has released its JAR file
>> (such that it may be deleted on Windows).
>>
>> Changes in this PR include:
>>
>> * Converting the test to JUnit 5
>> * Making it `@Parameterized` instead of using multiple jtreg runs
>> * Producing the sample class file using the ClassFile API instead calling
>> out to `javac` via ToolProvider
>> * Packaging the JAR using `JarOutputStream` instead of calling out to the
>> `jar` tool.
>> * Adding some code and method comments to support understading of what this
>> test does and why
>>
>> A nice benefit of this change is that it speeds up the runtime from ~15
>> seconds to ~1 second on my laptop.
>>
>> Unfortunately, this test relies on Windows to fail, that is something I plan
>> to look into separately.
>>
>> Verification: GHA tests pending. (No local Windows dev environment)
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Sanity check that the JarFileFactory caches are unpopulated at start of a
> test run
test/jdk/java/net/URLClassLoader/RemoveJar.java line 107:
> 105: Path jar = createJar();
> 106: String path = jar.toAbsolutePath().toString();
> 107: URL url = new URL("jar", "", "file:" +path + "!/" + subPath);
Suggestion:
URL url = new URL("jar", "", "file:" + path + "!/" + subPath);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22319#discussion_r1892210108
