On Mon, 19 May 2025 12:15:38 GMT, David Beaumont <d...@openjdk.org> wrote:
>> Adding read-only support to ZipFileSystem.
>>
>> The new `accessMode` environment property allows for readOnly and readWrite
>> values, and ensures that the requested mode is consistent with what's
>> returned.
>>
>> This involved a little refactoring to ensure that "read only" state was set
>> initially and only unset at the end of initialization if appropriate.
>>
>> By making 2 methods return values (rather than silently set non-final fields
>> as a side effect) it's now clear in what order fields are initialized and
>> which are final (sadly there are still non-final fields, but only a split of
>> this class into two types can fix that, since determining multi-jar support
>> requires reading the file system).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed test.
On a general note, all of these updated files will require a copyright year
update.
test/jdk/jdk/nio/zipfs/NewFileSystemTests.java line 239:
> 237: assertTrue(fs.isReadOnly());
> 238: if (!"First
> version".equals(Files.readString(fs.getPath("file.txt"), UTF_8))) {
> 239: fail("unexpected file content");
Nit, for this and the other new `fail` statements, it might be good to include
the unexpected file content in the fail message. If at all the test fails for
whatever reason, that additional detail usually help to quickly debug the
failures. Or maybe just replace the `if` followed by a `fail` with an
`assertEquals()` call.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25178#issuecomment-2890930980
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095657482
