RFR: 8004651 - TEST: java/util/logging/CheckLockLocationTest.java failed to delete file (win) (original) (raw)

Stuart Marks stuart.marks at oracle.com
Wed Dec 12 00:02:29 UTC 2012

Looks good!

Do you need someone to push this for you?

s'marks

On 12/11/12 3:04 PM, Jim Gish wrote:

A bit more cleanup as suggested:

http://cr.openjdk.java.net/~jgish/Bug8004651-CheckLockLocationTest-Windows-delete-file-fix/ <http://cr.openjdk.java.net/%7Ejgish/Bug8004651-CheckLockLocationTest-Windows-delete-file-fix/> Thanks, Jim On 12/10/2012 07:47 PM, Stuart Marks wrote: Hi Jim,

Catching IOException from delete() is a bit odd. The only thing in the delete() method that throws an IOE is the explicit throw of FileNotFoundException... so in that case we'd throw FNFE and then catch the IOE at the caller and print a warning. Would it be better to just print a warning from within the delete() method, and remove "throws IOException" ? There's only one other caller to delete() and it seems indifferent to this change. Now that we're no longer checking the message of FileSystemException, it's possible to change the instanceof check into a separate catch-clause of FileSystemException, which simply ignores that exception. The catch clause for IOException can be simplified to unconditionally wrap the IOE in a RuntimeException and rethrow it. Actually it's not clear to me that's even necessary since runTests() is declared to throw IOException, so we might not even need to catch IOE here at all; we can just let it propagate to the caller. Looks like similar simplifications apply to tests 2 and 4 as well. s'marks On 12/7/12 11:18 AM, Jim Gish wrote: Please review http://cr.openjdk.java.net/~jgish/Bug8004651-CheckLockLocationTest-Windows-delete-file-fix/

<http://cr.openjdk.java.net/%7Ejgish/Bug8004651-CheckLockLocationTest-Windows-delete-file-fix/>

Summary -- failure to delete a test log should be a warning instead of a failure. Also, while fixing this problem another one popped up -- not all platforms generate the same message in the FileSystemException ("Not a directory"), so removing the exception content check. Thanks, Jim -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 Oracle Java Platform Group | Core Libraries Team 35 Network Drive Burlington, MA 01803 jim.gish at oracle.com

More information about the core-libs-dev mailing list