On Fri, 11 Aug 2023 14:32:29 GMT, Alan Bateman <[email protected]> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comments
>
> test/jdk/java/io/FileDescriptor/Sync.java line 36:
>
>> 34: * @requires vm.continuations
>> 35: * @library /test/lib
>> 36: * @run main/othervm -XX:+UnlockExperimentalVMOptions
>> -XX:-VMContinuations Sync
>
> Running it with -XX:-VMContinuations is probably overkill here as it will be
> the same as the default case.
Agreed, removed.
> test/jdk/java/io/FileDescriptor/Sync.java line 83:
>
>> 81:
>> 82: File tmpFile = File.createTempFile("FileDescriptorSync2", "tmp");
>> 83: testWith(tmpFile);
>
> This will leave the file on the tmp file system. A try-finally to delete it
> might be best as we've had issues with temp file accumulating in long test
> runs.
Agreed. I think `try-with-resources` wrapper is cleaner here. See new commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291452045
PR Review Comment: https://git.openjdk.org/jdk/pull/15231#discussion_r1291457992
