Hi Sergey & Prahalad,

Thanks for your suggestions.
I have updated the test case to use finally block to delete the resources. Some 
of the changes previously present in test case are because of typo mistakes 
picked from another test case.
Please find the updated webrev:
http://cr.openjdk.java.net/~jdv/8183349/webrev.01/ 

Regards,
Jay

-----Original Message-----
From: Sergey Bylokhov 
Sent: Wednesday, July 12, 2017 12:08 AM
To: Prahalad Kumar Narayanan
Cc: 2d-dev@openjdk.java.net; Jayathirth D V
Subject: Re: [OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for 
jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and 
WriteAfterAbort.java

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.naraya...@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@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

Reply via email to