On Wed, 10 Jan 2024 17:37:11 GMT, Lance Andersen <lan...@openjdk.org> wrote:
> Thank you for the PR. Overall it looks good a few couple nit comments.
Thanks Lance, see e4a505fa073874824f247c20b76c3531a068ee32 for the latest
update following your review.
> test/jdk/jdk/nio/zipfs/TestPosix.java line 761:
>
>> 759: Files.write(zipFile, zip);
>> 760:
>> 761: // Verify that a read/synch roundtrip preserves the external
>> file attributes
>
> typo 'synch'
This comment was replaced with "Run the provided action on the ZipFileSystem"
following the rewrite described below.
> test/jdk/jdk/nio/zipfs/TestPosix.java line 770:
>
>> 768:
>> 769: // Verify calling Files.setPosixFilePermissions with current
>> permission set
>> 770: try (FileSystem fs = FileSystems.newFileSystem(zipFile,
>> ENV_POSIX)) {
>
> This should be a separate test IMHO
I've separated two tests `setPermissionsShouldPreserveRemainingBits` and
`setLastModifiedTimeShouldNotChangeExternalFileAttribute`, both using the
support method `assertExternalFileAttributeUnchanged`.
I agree this was much cleaner, since these are in fact separate and independent
(but perhaps overlapping) issues.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17170#issuecomment-1885367252
PR Review Comment: https://git.openjdk.org/jdk/pull/17170#discussion_r1447745607
PR Review Comment: https://git.openjdk.org/jdk/pull/17170#discussion_r1447747366
