DocumentTest->testCreateElementNeg, the conversion seems to be
incorrect. The original test, as the Javadoc stated, "This checks for
the constuctor of DOMException ". The new test however, only verifies
that an Exception is thrown.
Comments for tests such as testCreateAttributeNSNeg can be more
descriptive, for example, "Test for createAttributeNS method: verifies
that DOMException is thrown if reserved prefixes are used with an
arbitrary namespace name."
The original tests that made up DocumentTypeTest had comments that you
may want to copy over or rewrite as you did other tests.
The comment for EntityChildTest can be "Test DOM Parser: parsing an xml
file that contains external entities."
NodeListTest: @summary Verifies a bug found in jaxp1.0.1 and 1.1FCS.
After going out of bound, the last element of a NodeList returns null.
The bug has been fixed in jaxp 1.1.1 build.
Thanks,
Joe
On 4/1/2015 4:01 AM, Frank Yuan wrote:
Hi Joe and Lance
Many thanks for your review and comments!
I have added the comments to the tests as your suggestions except some
test method is very simple, the method name/code can present all
information, e.g. method testHasFeature in DomImplementationTest.java.
Please check my update in
http://cr.openjdk.java.net/~fyuan/8051559/webrev.01/
<http://cr.openjdk.java.net/%7Efyuan/8051559/webrev.01/>
As well, I understand your suggestion about the code comment, I will
think about how to add the comment on the viewpoint of the other code
reader in my future work, thank you!
Best Regards
Frank
*From:*huizhe wang [mailto:huizhe.w...@oracle.com]
*Sent:* Wednesday, April 01, 2015 1:14 PM
*To:* Lance Andersen
*Cc:* Frank Yuan; Core-Libs-Dev; Gustavo Galimberti
*Subject:* Re: Review request for JDK-8051559: JAXP function dom tests
conversion
Hi Frank,
I did see the request, just didn't have time to look at it.
I again agree with Lance, that these tests were written over 10 years
ago, it would be valuable to write down whatever understanding you
gained while converting the tests, same as the Astro application/test,
the goal of each test and how it works would all be helpful.
Basically, it would be nice to add some comment on @Test.
I tried the tests. They worked fine with my current build (with some
changes).
Thanks,
Joe
On 3/31/2015 7:10 AM, Lance Andersen wrote:
Hi Frank
On Mar 31, 2015, at 7:24 AM, Lance @ Oracle
<lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>> wrote:
Hi frank
Can you forward the other review request as I think I thought
they were the same and deleted it
Ignore this comment, the subjects were too similar but this is
what needed reviewed.
I will look at this again today
The tests overall look fine.
I still have the same comment WRT providing a simple comment
describing each test. The key point to remember is we want to
make it easier for someone to look at the test, understand what
you are trying to validate, and understand the coverage of the
tests. This will help future maintainers of the code. Comments
are just as important in test code as it is in implementation IMHO.
Best
Lance
Best
Lance
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance
Andersen| Principal Member of Technical Staff |
+1.781.442.2037 <tel:+1.781.442.2037>
Oracle Java Engineering
1 Network Drive <x-apple-data-detectors://34/0>
Burlington, MA 01803 <x-apple-data-detectors://34/0>
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
Sent from my iPad
On Mar 31, 2015, at 4:15 AM, Frank Yuan <frank.y...@oracle.com
<mailto:frank.y...@oracle.com>> wrote:
Hi Joe
Do you have any comment for dom suite co-location?
Best Regards
Frank
From: Frank Yuan [mailto:frank.y...@oracle.com]
Sent: Wednesday, March 25, 2015 5:46 PM
To: 'huizhe wang'; 'Core-Libs-Dev'
Cc: 'jibing chen'; 'Gustavo Galimberti';
sandeep.konch...@oracle.com
<mailto:sandeep.konch...@oracle.com>;
'Alexandre (Shura) Iline'
Subject: RE: Review request for JDK-8051559: JAXP function
dom tests
conversion
Hi, Joe and All
We are working on moving internal jaxp functional tests to
open jdk repo.
This is the dom suite. Would you please review these test?
Any comment will
be appreciated.
bug: https://bugs.openjdk.java.net/browse/JDK-8051559
webrev:
http://cr.openjdk.java.net/~fyuan/8051559/webrev.00/
<http://cr.openjdk.java.net/%7Efyuan/8051559/webrev.00/>
Thanks,
Frank
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance
Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
