Hi,
Two more things regarding the test
1. It was pointed out that you are using GPL+CP .. tests just use GPL
2. There's no @bug tag the test. In fact there's no bug ID here at all :-)
A bug ID is needed in order to push.
You have a JBS account now so could have created it yourself .. but
by the time I remembered that I'd already created it for you :-
https://bugs.openjdk.java.net/browse/JDK-8074587
But I did assign it to you :-)
-phil.
On 03/06/2015 02:03 PM, Laurent Bourgès wrote:
Phil,
Here is a new webrev:
http://cr.openjdk.java.net/~lbourges/webrev_Path2D_1/
<http://cr.openjdk.java.net/%7Elbourges/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
