[OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and WriteAfterAbort.java

[Sergey Bylokhov]

I guess the only change which is needed is to wrap the test() method in the try catch and 
add "Files.delete(file.toPath());fos.close();" to the finally block. It seems will have less code. It is unclear why the test was changed to the manual?


----- prahalad.kumar.narayanan at oracle.com wrote:

> Hello Jay
> 
> I looked into the changes. Please find my suggestions herewith.
> 
> . Refer to the javadocs of File class. It mentions that a directory
> could be deleted only if it's empty.
>   Hence, invoking directory.delete() will not work because a temp file
> would already exist in it.
>   Besides why should we delete a directory that is created by the test
> suite.
>       66         final File file = File.createTempFile("temp", ".img",
> directory);
>       67         directory.delete();
> 
> . Assuming the call to prepareWriteSequence fails, the subsequent call
> to Files.delete(...) at Line 78 will throw an exception.
>   FileOutputStream should be closed here as well. Similar to changes
> in Line 86.
>       78                 Files.delete(file.toPath());
> 
> Thank you
> Have a good day
> 
> Prahalad N.
> 
> --------------------
> From: Jayathirth D V 
> Sent: Monday, July 10, 2017 4:12 PM
> To: 2d-dev at openjdk.java.net
> Subject: [OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for
> jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and
> WriteAfterAbort.java
> 
> Hello All,
> 
> Please review the following fix in JDK10 :
> 
> Bug : https://bugs.openjdk.java.net/browse/JDK-8183349 
> Webrev : http://cr.openjdk.java.net/~jdv/8183349/webrev.00/ 
> 
> Issue : Temporary image files created in test case are not getting
> deleted after test execution is finished.
> Root cause : ImageOutputStream related to the file was closed
> previously and not FileOutputStream which was its parent.
> Solution : Closing the FileOutputStream allows us to delete temporary
> file. Also replaced file.deleteOnExit() with Files.delete().
> 
> Thanks,
> Jay