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

Chris Hegarty chris.hegarty at oracle.com
Tue May 30 09:44:05 UTC 2017


On 29/05/17 16:17, Felix Yang wrote:
> Hi Chris,
>
>     please review the updated webrev below. Comments inline.
>
> http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/

Thanks Felix.

> ..
>> 2) test/java/net/URLClassLoader/getresourceasstream/policy
>>
>>    Please add a copyright/header.
>>
> Checked existing test policy files, all of them have no
> copyright/header. So I hesitated to add here. Please confirm.

The code is not consistent, but please do add the copyright
header here ( we'll fix the other files at a later time ).

>> ...
>> 4) Why has the test for 5077773 simply been removed?
> As priorly commented, the test is probably not necessary.
> Because it actually quits immediately after prerequisite checking for
> "javax/swing/text/rtf/charsets/mac.txt". According with JDK-5077773, it
> is to cover endorsed scenarios, while endorsed mechanism has been
> removed in JDK 9.

Ok, sorry for not seeing the previous reply.

I am finished reviewing this. No need for an updated webrev, for
the license header.

-Chris.

> -Felix
>>
>> -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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>


More information about the core-libs-dev mailing list