RFR: 8322565 (zipfs) Files.setPosixPermissions should preserve 'external file attributes' bits [v2]
Eirik Bjørsnøs
eirbjo at openjdk.org
Wed Jan 10 18:11:40 UTC 2024
On Wed, 10 Jan 2024 17:37:11 GMT, Lance Andersen <lancea at 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
More information about the core-libs-dev
mailing list