Hello Sergey & Prahalad, Thanks for your reviews. I will check-in the changes.
Regards, Jay -----Original Message----- From: Sergey Bylokhov Sent: Saturday, July 15, 2017 2:21 AM To: Prahalad Kumar Narayanan; Jayathirth D V 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 +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.
