RFR 8166139/10, Refactor java/net shell cases to java

[Chris Hegarty]

Felix,

Thanks for taking this one. A few comments:


1) test/java/net/URLConnection/6212146/TestDriver.java
    Please check indentation around L81:

   80         Files.copy(testJar,
              ___________targetDir.resolve(ARCHIVE_NAME),
   81         ___________StandardCopyOption.REPLACE_EXISTING);


    Also, and more importantly, the ulimit that was executed in the shell
    is supposed to affect the potential file descriptor usage of the
    following java process. I believe that is not captured correctly
    in the TestDriver.java, since there is no parent/child relationship
    between the processes, no?


2) test/java/net/URLClassLoader/getresourceasstream/policy

    Please add a copyright/header.

3) getresourceasstream/TestDriver.java, CheckSealedTest.java, CloseTest.java

    If you statically import StandardCopyOption.REPLACE_EXISTING in a
    few places it may lead to a few shorter lines.

4) Why has the test for 5077773 simply been removed?

-Chris.


On 27/05/17 02:15, Felix Yang wrote:
> Hi Roger,
>
>     thanks for the comments. Please see the new webrev
>
> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.02/
>
> -Felix
> On 2017/5/27 3:50, Roger Riggs wrote:
>> Hi Felix,
>>
>> fyi, there is a new version of webrev.ksh that provides convenient
>> next and previous links.
>> http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh
>>
>>
>>
>> CloseTest.java: 66; checking WORK_DIR *after* calling setup() does not
>> make sense.
>>
>> Ditto: closetest/GetResourceAsStream.java
> Removed such check, since the test depends on several system
> properties("test.src", "user.dir"). It is not expected to run outside
> jtreg.
>
> -Felix
>>
>> URLConnection/6212146/Test.java:47 typo: ULR -> URL
>>
>> Regards, Roger
>>
>>
>> On 5/25/2017 11:14 PM, Felix Yang wrote:
>>> Hi Roger,
>>>
>>>     please review the updated webrev:
>>>
>>> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/
>>>
>>> Thanks,
>>> Felix
>>> On 2017/5/26 3:24, Roger Riggs wrote:
>>>> Hi Felix,
>>>>
>>>> closetest/CloseTest:
>>>>
>>>> 32: Please put the @modules directives together (and not repeat)
>>> Fixed
>>> -Felix
>>>>
>>>> 44: do not use wildcard imports  (reconfigure your IDE if necessary
>>>> to avoid accidents).
>>> Fixed and also organized imports in other test files
>>> -Felix
>>>>
>>>> 63: setup() is invoked twice...  remove 1 or explain why 2 calls
>>> Fixed
>>> -Felix
>>>>
>>>> 69, 78,90,91,... :  space in method call is not proper style
>>> Fixed and also formatted other history codes. Just format only
>>> without logic change.
>>> Not restricted to this test. I only formatted history codes "nearby",
>>> to avoid hiding real logic changes in the patch.
>>>
>>> -Felix
>>>>
>>>> sealing/CheckSealed.java:
>>>>  - Why is @test block removed?
>>>>     Should be converted to @run main CheckSealed
>>> There was a checksealed.sh. I replaced it with CheckSealedTest.java
>>> and declared @test there.
>>> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html
>>>
>>>
>>> BTW, you may noticed test/java/net/URLClassLoader/B5077773.* were
>>> removed. That is because corresponding prerequisite was removed even
>>> in JDK 8.
>> Perhaps then add a comment to CheckSealed.java indicating it is
>> compiled and executed by CheckSealedTest.java.
>>
>> Roger
>>
>>>
>>> -Felix
>>>>
>>>> Regards, Roger
>>>>
>>>>
>>>>
>>>> On 5/25/2017 4:08 AM, Felix Yang wrote:
>>>>> Hi there,
>>>>>
>>>>>    please review following patch to convert all shell cases under
>>>>> java/net to plain java codes.
>>>>>
>>>>> Webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.00/
>>>>>
>>>>> Bug:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8166139
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Felix
>>>>>
>>>>
>>>
>>
>