[9] RFR(S): 8164649: Cleanup of test java/nio/channels/FileChannel/Lock.java

[Alan Bateman]

On 23/08/2016 16:33, Langer, Christoph wrote:
>
> Hi,
>
> can I get a review for a test cleanup that I’ve made:
>
> https://bugs.openjdk.java.net/browse/JDK-8164649
>
> http://cr.openjdk.java.net/~clanger/webrevs/8164649.1/ 
> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8164649.1/>
>
> I was looking for a test that could reproduce the issue of 
> https://bugs.openjdk.java.net/browse/JDK-7146506 on AIX and found that 
> this test case inherently tests that bug as well. So I added a tag and 
> found some other potential for cleaning it up and making it a bit more 
> straightforward to understand JBut the behavior of the test itself 
> should not have changed.
>
>
No objection to updating this old test to use try-with-resources but I 
think needs a bit of formatting clean-up before pushing this.

attemptLock - can "fos" be changed "raf". Also if the original File f 
were retained then it will be much easier to read as:

File f = new File(s);
try (RandomAccessFile raf = new RandomAccessFile(f, "rw")) {
     :
}

The FileChannel can then be obtained inside the block.

test4 looks fine

test3 looks fine.

test2 looks fine.

test1 looks a bit messy, L89-90 in particular. Also L118 switches to /* 
*/ comments when // is used elsewhere. Also L65 makes it hard to see 
where the code in the try-finally is, it's inconsistent with the other 
tests.

That's all I have, no semantics issues.

-Alan










test2 looks fine.
test3 looks fine.




L89 and
L65 is an example where it's hard to see where the block starts. The 
other methods are easier to read.
L89 is really oddly formatted.

L89


L65 - very hard to see where the method starts so a blank line or put 
the { on the next line should make it easier to read.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160824/ec86132c/attachment.html>