(10) RFR of JDK-8181478, Refactor java/io shell tests to plain java tests

Hamlin Li huaming.li at oracle.com
Mon Jun 19 02:47:35 UTC 2017



On 2017/6/17 1:31, Paul Sandoz wrote:
>> On 14 Jun 2017, at 23:29, Hamlin Li <huaming.li at 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.li at 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



More information about the core-libs-dev mailing list