[JDK 11] RFR 8201528: Add new test to check for package versioning information in OpenJDK

[Chris Yin]

Hi, Mandy

Thanks lot for your suggestions, update webrev as below and comment inline, thanks

http://cr.openjdk.java.net/~xyin/8201528/webrev.02/ <http://cr.openjdk.java.net/~xyin/8201528/webrev.02/>


> On 9 Jun 2018, at 4:22 AM, mandy chung <mandy.chung at oracle.com> wrote:
> 
> 
> 
> On 6/8/18 12:07 AM, Chris Yin wrote:
>> Hi, Mandy
>> Many thanks for your detailed review and comments, updates new webrev
>> as below, and comment inline, thanks
>> webrev: http://cr.openjdk.java.net/~xyin/8201528/webrev.01/
> 
> 
> Thanks for adding the test description, that is very helpful.
> This is looking pretty good.  I have some suggestion on the test
> arguments that one can translate to the command-line easily.
> 
>  69  * PackageFromManifest runJar single
>  73  * PackageFromManifest runUrlLoader single
> 
> single means running with "testpack1.jar".  What about putting
> the JAR file name as the option which makes it very clear what
> is being tested.  Nit: "testpack1" could be confused with pack200
> and maybe simply test1.jar. How about

Fixed, thanks

> 
>   @run main PackageFromManifest runJar test1.jar
> 
>  77  * PackageFromManifest runJar 1
>  81  * PackageFromManifest runJar 2
> 
> These tests load two JAR files and load foo.Foo<n>.  What do you
> think to express:
> 
>  PackageFromManifest runJar test1.jar test2.jar foo.Foo1
>  PackageFromManifest runJar test1.jar test2.jar foo.Foo2
> 
>  85  * PackageFromManifest runUrlLoader 1
>  89  * PackageFromManifest runUrlLoader 2
> 
>  PackageFromManifest runUrlLoader test1.jar test2.jar foo.Foo1
>  PackageFromManifest runUrlLoader test1.jar test2.jar foo.Foo2

Used new style as you suggested, that looks exactly match with the description :)

> 
> Nit: I suggest to group the runType together. i.e. move line 34
> to above line 37. "<p>" not strictly needed as we won't generate
> the javadoc.  You can consider leaving it a blank line.

Sure, grouped runType and removed <p> (oh, it’s added by IDE during code reformat)

Thanks & Regards,
Chris

> 
> Mandy
>