On Thu, 14 Nov 2024 16:56:00 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> test/jdk/jdk/nio/zipfs/TestPosix.java line 236:
>>
>>> 234: }
>>> 235: return zfpv.readAttributes().group().getName();
>>> 236: } catch (UnsupportedOperationException | NoSuchFileException
>>> e) {
>>
>> In the prior code, I think `NoSuchFileException` would have been the cause
>> of the `SecurityException`, so the code would have returned `null`. Just
>> wondering why you changed it here.
>
> Interesting catch..
>
> My understanding is that the purpose of `expectedDefaultOwner` and
> `expectedDefaultGroup` is to mimic the behavior of `ZipFileSystem:initOwner`
> and `ZipFileSystem::initGroup`.
>
> The ZipFileSystem methods both check for
> `UnsupportedOperationException.getCause() instanceof NoSuchFileException`,
> while the TestPosix expect methods do not. This is why I added them, to make
> them consistent with the implementation code.
>
> This inconsistency is a however a preexisting issue. Attempting to fixing
> them here might just clutter this review. So I have reverted the catch for
> `NoSuchFileException`.
Ok, but your change did not check `UnsupportedOperationException.getCause()
instanceof NoSuchFileException`, it just checked if the exception was
`NoSuchFileException`.
In any case, I agree, best to preserve the existing code as much as possible.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842774624
