RFR: 8004651 - TEST: java/util/logging/CheckLockLocationTest.java failed to delete file (win)
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