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

Felix Yang felix.yang at oracle.com
Tue May 30 14:58:55 UTC 2017


Hi Chris,

    pushed, thank you very much!

-Felix
On 2017/5/30 17:44, Chris Hegarty wrote:
> 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 ).
Added.
>
>>> ...
>>> 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 net-dev mailing list