On Thu, 14 Mar 2024 19:38:48 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Alexey Ivanov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add copyright header
>
> test/jdk/javax/swing/plaf/basic/BasicDirectoryModel/ConcurrentModification.java
> line 211:
>
>> 209: private static void deleteFile(final Path file) {
>> 210: try {
>> 211: Files.delete(file);
>
> Should we try to delete all files first and then throw an exception if any?
I do try to remove all the files:
https://github.com/openjdk/jdk/blob/0d6be7a4e4368ea67382af4321b9483236276e5a/test/jdk/javax/swing/plaf/basic/BasicDirectoryModel/ConcurrentModification.java#L115-L120
https://github.com/openjdk/jdk/blob/0d6be7a4e4368ea67382af4321b9483236276e5a/test/jdk/javax/swing/plaf/basic/BasicDirectoryModel/ConcurrentModification.java#L202-L215
Probably, this can be chained via `handleException`. On the other hand, I don't
want to fail the test if cleaning up the files fails. However, now an exception
thrown from the `finally` block will override any exception thrown from the
test code, which is a bad thing, so I'll need to reconsider exception handling
during clean-up.
If test fails to clean up the files it created, jtreg will remove them. It is
the reason why I use the current directory as the base, which is the `scratch`
directory when run in jtreg.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18109#discussion_r1526123644
