On Sun, 16 Apr 2023 08:39:32 GMT, Alan Bateman <[email protected]> wrote:
>> added validation for other file being null but also kept the existing
>> validation of the message
>
> Okay, although I assume this test will fail if it throws a more general
> IOException (it's allowed to do that) or there is any adjustment to the error
> message.
Yes, in the unlikely event that either FileSystemException or the code below
from zipfs is changed, then yes the error message could evolve
if (e.isDir())
throw new FileSystemException(getString(path), null, "is a
directory");
I do not have a strong preference once way or the other and am happy to remove
the message validation if that is what you prefer. I kept the validation as
the original issue was surrounding the actual message being generated which
aligns with the text included by zipfs when it threw the exception when
obtaining the inputstream
I think it is worth mentioning that we have other tests that also validate
exception messages(and we recently updated a couple zip tests because the
message changed). So if we are going to move in the direction of not
validating exception messages in tests, we should probably consider adding
verbiage to the developers guide to provide that guidance and be consistent
within the test suite
Let me know your preference and thank you for your thoughts as it is a useful
discussion
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13482#discussion_r1167813939
