Phil,
Here is a new webrev:
http://cr.openjdk.java.net/~lbourges/webrev_Path2D_1/
See my comments below:
> you placed the test in the java.awt.geom package.
>>
>> 25 package java.awt.geom;
>>
>> and are accessing internals of that package.
>>
>> In jigsaw/modular mode that won't even compile.
>>
>
> Ok it is annoying:
> as all Path2D fields are package protected, I designed the test using
> direct access to any fields ...
>
>
>> So the test should go in the anonymous package and avoid accessing
>> internals.
>> It should be possible to use just public API to verify the arrays of a
>> shape
>> being cloned are trimmed .
>>
>
> No, it is not possible to use Shape API to access arrays nor fields
> (numTypes ...):
> only getPathIterator() could give me data but it won't tell me if the
> underlying arrays or fields are correct.
>
>
> That is true ..
>
> Well, if you need it to be in java.awt.geom, I think even today you'll
> find it won't work
> unless you jump through some jtreg hoops to install it on the
> bootclasspath.
> I think its something like "@run main/othervm -Xbootclasspath/a:. "
> And later in the modular JDK it will need to be modified again.
>
> I'd say either update the test to work with jtreg today - and test it to
> be sure,
> or provide the test without an @test tag, or with an @ignore tag, so
> people can
> still manually verify it but the harness won't run it.
>
I removed the @test tag but added comments indicating to run the test
manually.
>
>
> Maybe I could use introspection to getDeclaredField(name) and
> setAccessible(true) to get internal data.
>
>
> That won't work either. So maybe this is a "noreg-hard" or "noreg-cleanup"
> bug.
> We add those labels to the JBS/JIRA bug when something isn't testable.
>
Nevermind.
> Any idea or utility class I could use
>
>
>
>> Why is it necessary to explicitly add the call to super(); ?
>>
>> 223 super();
>>
>
> I agree it is not necessary but it explicitely says that I use the empty
> constructor:
>
> /**
> * Constructs a new empty {@code Path2D} object.
> * It is assumed that the package sibling subclass that is
> * defaulting to this constructor will fill in all values.
> *
> * @since 1.6
> */
> /* private protected */
> Path2D() {
> }
>
>
> If we all did this, all of the time, there'd be a lot of extra lines in
> the code, that the compiler
> would fill in for us anyway.
>
I removed the superfluous super() calls.
Thanks for your review,
Laurent
