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

mandy chung mandy.chung at oracle.com
Fri Jun 8 20:22:22 UTC 2018



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

    @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

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.

Mandy



More information about the core-libs-dev mailing list