On Fri, 26 Jul 2024 04:30:16 GMT, Jaikiran Pai <[email protected]> wrote:
>> SendaoYan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 1. Just catch the IOException here when getting the FileStore and skip the
>> test instead of checking for specific exception messages. 2. Throw a
>> org.testng.SkipException instead print and return
>
> test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java line 217:
>
>> 215: } catch (IOException e) {
>> 216: e.printStackTrace();
>> 217: throw new SkipException("WARNING: IOException occur.
>> Skipping testDumpDirNotWritable test.");
>
> Nit: "occurred" instead "occur". Additionally, I would suggest even the
> exception's toString() in that message just to provide additional context at
> the location wherever this will get reported outside of a .jtr (if at all).
> So something like:
>
> throw new SkipException("WARNING: IOException occurred: " + e + ", Skipping
> testDumpDirNotWritable test.");
Thanks for your advice and review again.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19905#discussion_r1692570083
