review request for 7022624, convert java.io test to use try-with-resources

Alan Bateman Alan.Bateman at oracle.com
Tue Mar 1 09:46:50 UTC 2011


Stuart Marks wrote:
> Here's a small webrev with changes to a handful of java.io tests to 
> use TWR.
>
> http://cr.openjdk.java.net/~smarks/reviews/7022624/webrev.0/
>
> I have a few minor questions:
>
> * test/java/io/File/SetLastModified.java
>
> Should the channel corresponding to a stream be considered a separate 
> resource, or is it more like another object that represents the "same" 
> resource? Since closing one closes the other, I treated them as peers 
> and I didn't unroll the FileOutputStream and its channel into separate 
> resource variables. However, I'm flexible on this.
The associated channel is connected to the same file as the input 
stream. In theory it might be possible for getChannel to fail with a 
virtual machine error (stack overflow or OOME for example) but for this 
test I think what you have is fine.

>
> * test/java/io/OutputStreamWriter/Encode.java
>
> Pretty clearly a ServerSocket is a distinct resource from a Socket 
> returned from the accept() call. However, does Socket.getInputStream() 
> represent a distinct resource from the Socket? In this case it seemed 
> most sensible to unroll them into separate resource variables, but 
> again I could go either way on this.
I wouldn't bother but would instead reduce this down to three resources, 
maybe:

try (ServerSocket listener = ss;
      Socket s = listener.accept();
      BufferedReader reader = new BufferedReader(new 
InputStreamReader(s.getInputStream()))
{
    ...
}

While you are there, I assume ss should be final. Also might be good to 
put a try/finally around the encode at L50-52 to ensure that disconnect 
is called.

>
> * test/java/io/PrintStream/FailingConstructors.java
>
> Please check over my NIO usage here. Hm, I did more conversion here 
> than in the other FailingConstructors tests (see webrev for 7021209). 
> Maybe I should go back and fix the others....
Looks fine to me.

>
> * 
> test/java/io/Serializable/evolution/RenamePackage/install/SerialDriver.java 
>
> * 
> test/java/io/Serializable/evolution/RenamePackage/test/SerialDriver.java
>
> Odd, seems like mostly duplicate code....
I don't know the history of that test but it does appear that it could 
have been written to generate the class to both packages rather than 
duplicating.

I've looked at the other tests in the webrev and all looks okay to me.

-Alan.





More information about the core-libs-dev mailing list