On 2017/6/17 1:31, Paul Sandoz wrote:
On 14 Jun 2017, at 23:29, Hamlin Li <huaming...@oracle.com> wrote:

Hi Alan, Paul,

Thank you for review, new webrev at: 
http://cr.openjdk.java.net/~mli/8181478/webrev.01/

Please also check my comments inline.


On 2017/6/15 1:28, Alan Bateman wrote:
On 14/06/2017 18:20, Paul Sandoz wrote:
On 12 Jun 2017, at 01:00, Hamlin Li <huaming...@oracle.com> wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8181478

webrev: http://cr.openjdk.java.net/~mli/8181478/webrev.00/

It took me a few moments to grok the NonExistentDriver behaviour. If i got it 
correct, this is checking that deleteOnExit is working correctly?

If so it might be clearer to make this is a separate test that is run as a 
driver and a test with different arguments performing the action and then the 
checking.
Agree, it's not that clear. And I just found out there is another test 
java/io/File/DeleteOnExit.java testing the File.deleteOnExit. so I just remove 
NonExistentDriver.java and related code.
Do you still require the following:

   48     static File nonDir = new File("x.Basic.nonDir");
...
   93         nonDir.delete();
...
  133         if (!nonDir.mkdir()) {
  134             fail(nonDir, "could not create");
  135         }
  136         if (!nonDir.exists() || !nonDir.isDirectory()) {
  137             fail(nonDir, "not created");
  138         }

since it is now just duplicating the assertions from the results of a call to 
show.
Hi Paul,

Thank you for detailed review.
I'm not sure if I understand you correctly, I think it still needs to verify the creation of a directory, so I merged the test of dir and nonDir in new webrev: http://cr.openjdk.java.net/~mli/8181478/webrev.02/

Thank you
-Hamlin

Paul.

I agree. Also in FileOpenTest then it can use APIs to set the hidden attribute, 
no need to launch attrib.exe each time.
This is a test running only on windows.
I'm not sure if I understand you correctly. In new patch I use Files.setAttribute(path, 
"dos:hidden", true/false); to hide/un-hide a file, and use File.setReadOnly() 
to make a file read only.

Thank you
-Hamlin
-Alan

Reply via email to