Hi Frank,
Looks good.
Since you've gone through the original test/the astro application, and
refactored it into a TestNG test, you probably have other insight too.
If so, it would be nice to add them as well.
In terms of report, yes, I can see the output files displayed along with
process id. Thanks for doing that. Not as good as the old test report,
which lists test mode/query ids with status, but acceptable since it
prints out the process and query ids that helps identifying the exact test.
I have always wondered, but thought not so important, that in its
current (TestNG) form, the test report will show test cases (thus fewer
tests than the original test suite). For example, the AstroApp reports
10 tests and 39 test cases, while the new 5. Console display is quite
different from the html report as well. For example, on the console, the
test results are: passed: 70, but the html report shows 371 tests
passed. What are your thoughts on this?
Thanks,
Joe
On 3/27/2015 2:43 AM, Frank Yuan wrote:
Hi Joe and Lance
Thank you very much for your review! It's very good comment!
According to your comments, I made the following changes:
1. Rename the output file name to be easier understanding.
2. Print the output file name to be easier debugging.
3. Add comment for AstroProcessor class
4. Add comment in AstroTest to describe the test, some are from astro
application design document.
Besides these new adding comments, there are many comments in astro
classes(in libs/test/astro), which are from the original source
files. And the current tests prints information to the standard out,
which is redirected to .jtr file by jtreg, so there is not any other
trace file now. USER_DIR is the scratch dir in deed, it is property
user.dir in jtreg. The new webrev is at
http://cr.openjdk.java.net/~fyuan/8051560/webrev.01/
<http://cr.openjdk.java.net/%7Efyuan/8051560/webrev.01/>
Btw, I also sent out review request for JDK-8051559: JAXP function dom
tests minutes later than this review request, I am afraid you may miss
it, reminder here:)
Best Regards
Frank
*From:*huizhe wang [mailto:huizhe.w...@oracle.com]
*Sent:* Friday, March 27, 2015 9:02 AM
*To:* Lance Andersen
*Cc:* Frank Yuan; 'Core-Libs-Dev'; 'Gustavo Galimberti'
*Subject:* Re: Review request for JDK-8051560: JAXP function astro
tests conversion
I second Lance. The Main of the original astro had Javadocs and
developer comments. Probably more important is that you've completely
changed the main classes (TestDriver and Main), which looks good,
however, the original classes contained a lot of information on what
each test does and how it works that seem to have all been lost.
The suite's README and build.xml may also contain information that is
worth keeping. Some of them may be no longer valid, in which case, it
may be helpful to describe the change. For example, the log files
described in README were useful for debugging. It would be good to
explain where to find the debug info in your new design.
While scanning the test, I see that you are creating temporary output
files under USER_DIR. Is that intended? JAXP processors do not
necessarily open them with the DELETE_ON_CLOSE option. I thought in
previous tests, you were creating them in the scratch directory.
Thanks,
Joe
On 3/26/2015 12:08 PM, Lance Andersen wrote:
Hi Frank,
Overall these look fine. I would suggest adding a simple comment
to describe the tests that do not have one to give a basic intent
of the test to make it easier for someone to understand if they
are new.
Best
Lance
On Mar 25, 2015, at 5:34 AM, Frank Yuan <frank.y...@oracle.com
<mailto:frank.y...@oracle.com>> wrote:
Hi, Joe and All
We are working on moving internal jaxp functional tests to
open jdk repo.
This is the astro suite. Would you please review these test?
Any comment
will be appreciated.
bug: https://bugs.openjdk.java.net/browse/JDK-8051560
webrev: http://cr.openjdk.java.net/~fyuan/8051560/webrev.00/
<http://cr.openjdk.java.net/%7Efyuan/8051560/webrev.00/>
AstroTest is the primary test in this suite, it transforms an
xml file(which
includes astro data) with several xsl files, sets different
filtering
condition by these xsl files and different filtering range,
finally compares
the result with golden files.
And there are 5 permutations of InputSourceFactory and
FilterFactory(I uses
template method pattern for the variant FilterFactoryImpls), each
permutation will be applied to above transforming processes.
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>
