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

Roger Riggs Roger.Riggs at Oracle.com
Fri May 26 19:30:28 UTC 2017


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