On Tue, 16 Feb 2021 11:37:05 GMT, Evan Whelan <[email protected]> wrote:
>> Hi,
>>
>> Please review this fix for JDK-8252883. This handles the case when an
>> AccessDeniedException is being thrown on Windows, due to a delay in deleting
>> the lock file it is trying to write to.
>>
>> This fix passes all testing.
>>
>> Kind regards,
>> Evan
>
> Evan Whelan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8252883: Doc cleanup, code formatting and throwing exceptions instead of
> catching
Thanks for taking on the changes. Still some comments...
test/jdk/java/util/logging/FileHandlerAccessTest.java line 75:
> 73: handler.close();
> 74: } catch (Exception e) {
> 75: throw new RuntimeException(e);
Will throwing an exception in a thread (if happening in the Thread created at
line 60) cause jtreg to fail? Or will the exception simply be discarded?
test/jdk/java/util/logging/FileHandlerAccessTest.java line 98:
> 96: childProcess.waitFor();
> 97: } catch (Exception e) {
> 98: throw new RuntimeException(e);
Same holds there as well. Maybe you could make an experiment with a dummy test
to see whether the exception will make the test fail.
test/jdk/java/util/logging/FileHandlerAccessTest.java line 96:
> 94: System.out.println(name + "\t|" + line);
> 95: }
> 96: childProcess.waitFor();
Should you be checking the status returned by `waitFor` and fail the test if
it's not `0`?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2572
