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.
