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

[Langer, Christoph]

Hi Alan,

thanks for looking into this.

See my new vebrev: http://cr.openjdk.java.net/~clanger/webrevs/8164649.2/

I've implemented all suggestions, except for line 89 where I don't know how it should be done better...

Thanks
Christoph

From: Alan Bateman [mailto:Alan.Bateman at oracle.com]
Sent: Mittwoch, 24. August 2016 18:46
To: Langer, Christoph <christoph.langer at sap.com>; nio-dev at openjdk.java.net
Subject: Re: [9] RFR(S): 8164649: Cleanup of test java/nio/channels/FileChannel/Lock.java




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 :) But 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/61099562/attachment-0001.html>