(10) RFR of JDK-8166142: Refactor java.io.serialization shell tests to java

[Roger Riggs]

Hi Hamlin,

Looks good.  On to the next.

Roger


On 5/31/17 9:52 PM, Hamlin Li wrote:
> Hi Roger,
>
> Thank you for detailed reviewing, fixed as you suggested except below 
> comment:
>
>     81-83/93/95:  use  SHARE.resolve(xxx).toString to compute the paths.
>
> As the code is constructing a class path with 2 paths rather than 
> constructing a path.
right, my mistake.
>
> new webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.03/
>
> Thank you
>
> -Hamlin
>
>
> On 2017/6/1 0:47, Roger Riggs wrote:
>> Hi Hamlin,
>>
>> RenamePackageTest.java:
>>   - 48,61:  rename "sutup" -> "setup"
>>
>> 81-83/93/95:  use  SHARE.resolve(xxx).toString to compute the paths.
>>
>> ClassPathTest.java:
>> 42: add a space before "{"
>>
>> 56:  Generally, exception messages should not end in "."; they are 
>> phrases
>> so they could be printed with context.
>> In this specific case it is not important, just a pattern to follow 
>> consistently.
>>
>> Thanks, Roger
>>
>> On 5/26/2017 9:14 PM, Hamlin Li wrote:
>>> Hi Roger,
>>>
>>> Thank you for catching it, new webrev: 
>>> http://cr.openjdk.java.net/~mli/8166142/webrev.02/
>>>
>>> Thank you
>>>
>>> -Hamlin
>>>
>>>
>>> On 2017/5/27 3:30, Roger Riggs wrote:
>>>> Hi Hamlin,
>>>>
>>>> SubclassTest.java:37: typo" excepiton" -> exception
>>>>
>>>>
>>>> SubclassDataLossTest.java:
>>>> 103/104:  Adding the class loader close() calls isn't very 
>>>> effective since if there is an exception they are not
>>>> closed and the creation in a static initializer is mismatched with 
>>>> main() code.
>>>>
>>>> It would be cleaner to open in main() using try-with-resources and 
>>>> close them in a finally clause.
>>>> And pass the loaders to the test function.
>>>> But for simplicity, perhaps just leave out the new calls to 
>>>> ldr1/2.close.
>>>>
>>>> (There is something odd about the webrev links, the nagivation 
>>>> links don't work as expected. but that's transient)
>>>>
>>>> Thanks, Roger
>>>>
>>>>
>>>>
>>>> On 5/26/2017 12:26 AM, Hamlin Li wrote:
>>>>> Hi Roger,
>>>>>
>>>>> Thank you for detailed review. Fixed as you suggested, new webrev: 
>>>>> http://cr.openjdk.java.net/~mli/8166142/webrev.01/
>>>>>
>>>>> Thank you
>>>>>
>>>>> -Hamlin
>>>>>
>>>>>
>>>>> On 2017/5/26 2:54, Roger Riggs wrote:
>>>>>> Hi Hamlin,
>>>>>>
>>>>>> For imports they should import specific classes, wildcards are 
>>>>>> not used.
>>>>>>
>>>>>> maskSyntheticModifier/Test.java
>>>>>> consTest/Test.java
>>>>>> deserializeButton/Test.java
>>>>>>
>>>>>>  test/java/io/Serializable/packageAccess/Driver.java: the name 
>>>>>> "Driver" is not descriptive and should be.
>>>>>>
>>>>>> Each Driver.java should have a different and 
>>>>>> functional/descriptive name.
>>>>>>
>>>>>> Better yet, there should be a single program that creates jar 
>>>>>> files using command line arguments
>>>>>> that can be invoked depending on the test.
>>>>>>
>>>>>> There is a mix of Driver's copying files vs the test program 
>>>>>> copying; can that be made more uniform.
>>>>>>
>>>>>> Test.java files should have descriptive/functional names. (Even 
>>>>>> if the duplicating the directory)
>>>>>>
>>>>>> Regards, Roger
>>>>>>
>>>>>>
>>>>>> On 5/24/2017 5:09 AM, Hamlin Li wrote:
>>>>>>> Would you please review the below patch?
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8166142
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.00/
>>>>>>>
>>>>>>>
>>>>>>> Thank you
>>>>>>>
>>>>>>> -Hamlin
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>