(10) RFR of JDK-8166142: Refactor java.io.serialization shell tests to java
Hamlin Li
huaming.li at oracle.com
Tue May 30 23:34:19 UTC 2017
Ping.
Thank you
-Hamlin
On 2017/5/27 9:14, 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
>>>>>
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list