Review request for JDK-8051710: Convert JAXP function tests: javax.xml.jaxp14.* to jtreg (testng) tests
lance.andersen at oracle.com
Wed Jan 28 21:46:07 UTC 2015
Overall they look fine. Minor comments that you can address when you have time but should not stop you pushing your changes/additions
TransformTest - just have setup throw Exception as you really don't need the granularity because your are not doing anything with it
XPathFactoryTest - Please be consistent as to whether you are using javadoc comments or not to introduce methods (i.e. clean up old or new tests)
DataProvider invalid-parameterss - seems to be the same in multiple classes so I would move it to the base class and override when needed
SchemaFactoryTest, ValidateHandlerTests, ValidatorTest - When time permits, I would add a brief comment for each test
On Jan 28, 2015, at 6:18 AM, Frank Yuan <frank.yuan at oracle.com> wrote:
> Hi Joe
> I have moved the jaxp14 tests to the corresponding packages, would you like
> to have a check? http://cr.openjdk.java.net/~fyuan/8051710/webrev.01/
> There is an issue, SchemaFactoryTest is merged to
> validation.SchemaFactoryTest, but the validation package(Lance reviewed it)
> is waiting to push. I am not sure how to handle for your convenience, so
> this webrev also includes validation package. Maybe we can wait for pushing
> validation, or push this patch for both jaxp14 and validation if you think
> these code change is ok.
> Best Regards
> -----Original Message-----
> From: huizhe wang [mailto:huizhe.wang at oracle.com]
> Sent: Wednesday, January 28, 2015 12:44 PM
> To: Frank Yuan
> Cc: 'Lance Andersen'; 'Core-Libs-Dev'; 'jibing chen'; 'Gustavo Galimberti';
> sandeep.konchady at oracle.com; 'Alexandre (Shura) Iline'
> Subject: Re: Review request for JDK-8051710: Convert JAXP function tests:
> javax.xml.jaxp14.* to jtreg (testng) tests
> On 1/27/2015 7:06 PM, Frank Yuan wrote:
>> Thank you, Joe.
>> I will sort the tests as your suggestion, and have 3 questions to
>> with you:
>> 1. Should I also sort the gap tests?
>> 2. Should I put astro suite at side of auctionportal?
> Ah, in that case, you may put gap tests alongside the auctionportal as well.
>> 3. If a package has very few test, e.g. I may put XMLEventFactoryTest
>> something else in javax.xml.stream.ptest, is it ok? (I would rename
>> XMLEventFactoryTest as its small coverage)
> Sounds good to me.
>> Best Regards
>> -----Original Message-----
>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
>> Sent: Wednesday, January 28, 2015 10:27 AM
>> To: Frank Yuan
>> Cc: 'Lance Andersen'; 'Core-Libs-Dev'; 'jibing chen'; 'Gustavo
>> sandeep.konchady at oracle.com; 'Alexandre (Shura) Iline'
>> Subject: Re: Review request for JDK-8051710: Convert JAXP function
>> javax.xml.jaxp14.* to jtreg (testng) tests
>> Hi Frank,
>> Nice refactoring the original tests, esp. the TransformerTest!
>> jaxp14 is legacy in the jaxp standalone world. While we are at this,
>> want to move these tests to their relevant packages since in the JDK
>> jaxp14 is no longer relevant (jaxp 1.4 was integrated into JDK 6). As
>> you've already split FactoryTest into various Factory tests, you may
>> them a bit thin in terms of test coverage now that they are named
>> *FactoryTest since they cover just one of the two newInstance methods.
>> I would think it makes sense to move them into / combine with the
>> Factory tests of their own packages.
>> On 1/27/2015 1:09 AM, Frank Yuan wrote:
>>> Hi, Joe, Lance and All
>>> We are working on moving internal jaxp functional tests to open jdk
>>> This is the jaxp14 suite. Would you please review these test? Any
>>> will be appreciated.
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8051710
>>> webrev: http://cr.openjdk.java.net/~fyuan/8051710/webrev.00/
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com
More information about the core-libs-dev