Hi Fernando,
While you add the comment (about compile/run), you may want to update
the original comment about the test as well as it mentioned "see run.sh"
that is now gone.
A minor comment on the header: a space between the years would be good,
that is "2014,2020," -> 2014, 2020,
-Joe
On 5/8/2020 10:49 AM, Fernando Guallini wrote:
Sure, I will add some of the explanation as a comment. Thank you Daniel!
On 8 May 2020, at 18:39, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
Hi Fernando,
Ah - I see the keypoint is the:
39 * @clean org.w3c.dom.*
OK, I missed that. Then I agree - ignore my previous comments.
Maybe the explanations below should be added to the test in some
comment to help future maintainers (and reviewers).
best regards,
-- daniel
On 08/05/2020 18:19, Fernando Guallini wrote:
Hi Daniel and Alan,
@compile/module=java.xml was my first try, but for the nature of this test, it
didn't work. The reason is that the original shell test does the following:
- Compiles it’s own version of Node and Document interfaces
- Compiles DocumentImpl patching java.xml with those 2 interfaces
- Executes the AbstractMethodErrorTest patching the java.xml module *only with
DocumentImpl* patch(not including Document and Node)
It does that by keeping the patches output in different folders. That’s
important otherwise AbstractMethodErrorTest doesn’t compile, because it
references some methods not declared in its custom interfaces, and it seems to
be coded this way to reproduce the original bug. That is also the reason why I
added the *@clean* command to remove Document and Node *before* running the
test.
But when using *@compile/module=java.xml*, under the hood, JTREG generates a folder
named “patches/java.xml/..” including all the compiled classes under it.
Unfortunately, the temporary interfaces can’t be removed because *@clean* does not
know how to resolve the path "/patches/java.xml/..”.
To sum up, this test creates a temporary java.xml patch that needs to be
ignored after compiling *DocumentImpl. *I am using —patch-module because it’s
more flexible than @compile/module
*
*
Hope I explained myself!
On 8 May 2020, at 15:49, Daniel Fuchs <daniel.fu...@oracle.com
<mailto:daniel.fu...@oracle.com>> wrote:
On 08/05/2020 15:40, Alan Bateman wrote:
31 /*
32 * @test
33 * @bug 8035437
34 * @summary Tests that java.lang.AbstractMethodError is not thrown when
35 * serializing improper version of DocumentImpl class.
36 * @library /test/lib
* @modules javax.xml/org.w3c.dom
* javax.xml/com.sun.org.apache.xerces.internal.dom
40 * @run main/othervm --patch-module java.xml=${test.class.path}
AbstractMethodErrorTest
41 */
(not 100% sure the @modules is even needed)
I wouldn't expect to need --patch-module here. Instead maybe it could be changed to use
@compile/module=java.xml ... and jtreg should compile and run the overrides "as
if" they are in the java.xml module. There are a couple of examples of this in the
test suite that might help get this going. No need for javax.xml/org.w3c.dom as that
package is already exported.
Right. Copy paste error. The --patch-module shouldn't be needed
anywhere. Good point about @compile - the main class
AbstractMethodErrorTest is not in the patched module, so
the patched classes may not get compiled if you don't force
their compilation.
Thanks for the correction Alan!
-- daniel
