On Mon, 20 Sep 2021 11:28:10 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Hi all,
>>
>> Please review this patch which addresses the issue where Zip FS will throw a
>> UOE instead of returning null when Files.getFileAttributeView() is invoked
>> and the view not supported.
>>
>> Mach5 tiers1 - tier3 are clean.
>>
>> Best
>> Lance
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java line 716:
>
>> 714: return (V)new ZipPosixFileAttributeView(this,true);
>> 715: }
>> 716: return (V) null;
>
> I assume (V) isn't needed here.
No, it is not and can also probably be removed from
open/src/java.base/unix/classes/sun/nio/fs/UnixFileSystemProvider.java at some
point
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 59:
>
>> 57: public void setup() throws IOException {
>> 58: Files.deleteIfExists(ZIP_FILE);
>> 59: Entry e0 = Entry.of(ZIP_ENTRY, ZipEntry.DEFLATED,
>
> Is there a reason why this is named "e0"?
Not really. Historically that was the variable name format used in some of the
Zip FS tests. I changed the name to `entry`
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 82:
>
>> 80: return new Object[][]{
>> 81: {Map.of()},
>> 82: { Map.of("enablePosixFileAttributes", "true")}
>
> Minor nit, inconsistent formatting with L81 vs. L82.
Fixed, thanks for catching that.
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 89:
>
>> 87: * Verify that Files.getFileAttributeView will not throw
>> 88: * UnsupportedOperationException when the attribute view
>> 89: * PosixFileAttributeView is not available
>
> Might be clearer to say that it checks that Files.getFileAttributeView does
> not throw an exception (no need to mention UOE) when PosixFileAttributeView
> is not supported.
Updated per your suggestion!
> test/jdk/jdk/nio/zipfs/testng/test/PosixAttributeViewTest.java line 95:
>
>> 93: @Test(dataProvider = "zipfsMap")
>> 94: public void testPosixAttributeView(Map<String, String> env) throws
>> Exception {
>> 95:
>
> Spurious blank line.
Removed.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5579
