On Mon, 12 May 2025 10:01:05 GMT, David Beaumont <d...@openjdk.org> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fixed test.
>
> test/jdk/jdk/nio/zipfs/Utils.java line 52:
>
>> 50: */
>> 51: static Path createJarFile(String name, String... entries) throws
>> IOException {
>> 52: Path jarFile = Paths.get(name);
>
> Previous tests always had the test file called the same thing.
> Note that in jtreg tests, the file of the same name often already exists in
> the test directory.
> I tried adding a "ensure file doesn't already exist" check and it fails lots
> of tests.
It's interesting that we have several places in the test which use this
`Utils.createJarFile()` and pass it a test specific JAR file name and yet this
utility was ignoring the passed name hard coding the name to `basic.jar`. It
appears to be an oversight and your fix here I believe is a good thing.
> I tried adding a "ensure file doesn't already exist" check and it fails lots
> of tests.
I think it's OK in its current form to allow the file to be overwritten. Plus,
you have also updated the javadoc of this test utility method to explicitly
state its current behaviour, which I think is enough to let the call sites know
how this behaves.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095672596
