+1
On 12.07.2017 22:27, Prahalad Kumar Narayanan wrote:
Hello Jay
Minor changes have been added.
Looks good.
Thank you
Have a good day
Prahalad N.
-----Original Message-----
From: Jayathirth D V
Sent: Wednesday, July 12, 2017 4:05 PM
To: Sergey Bylokhov; Prahalad Kumar Narayanan
Cc: 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for
jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and
WriteAfterAbort.java
Hello All,
I added null check in finally block of test case after getting inputs from
similar change in JDK-8183341.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8183349/webrev.02/
Thanks,
Jay
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, July 12, 2017 12:50 PM
To: Prahalad Kumar Narayanan
Cc: Jayathirth D V; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for
jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and
WriteAfterAbort.java
+1
----- prahalad.kumar.naraya...@oracle.com wrote:
Hello Jay
The changes look good and contains lesser code than the earlier
revision.
Thanks
Have a good day
Prahalad N.
-----Original Message-----
From: Jayathirth D V
Sent: Wednesday, July 12, 2017 11:46 AM
To: Sergey Bylokhov; Prahalad Kumar Narayanan
Cc: 2d-dev@openjdk.java.net
Subject: RE: [OpenJDK 2D-Dev] [10] RFR JDK-8183349: Better cleanup for
jdk/test/javax/imageio/plugins/shared/CanWriteSequence.java and
WriteAfterAbort.java
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
--
Best regards, Sergey.
