On Fri, 11 Aug 2023 14:32:29 GMT, Alan Bateman <al...@openjdk.org> 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

Reply via email to