RFR: 8166398: CatalogSupport tests need to be fixed -> Re: RFR: 8166220: Catalog API: JAXP XML Processor Support - add StAX test coverage

[Joe Wang]

Hi Daniel,

Here's the fix for the test issues, a couple of errors including missing 
handler in StAX test, and dtd was mistakingly typed as xsd . The root 
cause is a debugging assertEquals method that printed out debugging 
message without throwing Exception. I've searched the jaxp/test to make 
sure this is the only case where this is used.

While fixing the tests, also refactored the code so that the use-catalog 
is checked as a top condition, removed a ObjectFactory call in the course.

JBS: https://bugs.openjdk.java.net/browse/JDK-8166398
webrev: http://cr.openjdk.java.net/~joehw/jdk9/8166398/webrev/

Thanks,
Joe

On 9/20/16, 11:22 AM, Joe Wang wrote:
>
>
> On 9/20/16, 2:20 AM, Daniel Fuchs wrote:
>> Hi Joe,
>>
>> test/javax/xml/jaxp/unittest/catalog/CatalogSupportBase.java
>>
>>  603     /**
>>  604      * Returns the text of the first element found by the reader.
>>  605      * @param streamReader the XMLStreamReader
>>  606      * @return the text of the first element
>>  607      * @throws XMLStreamException
>>  608      */
>>  609     String getText(XMLStreamReader streamReader, int type) 
>> throws XMLStreamException {
>>
>> It would be good to update the javadoc of this method (in particular
>> document the new 'type' parameter) so that future maintainer
>> can understand what that method is supposed to do.
>
> Thanks Daniel!  I'll fix this.
>
> As I went through the tests again, I found there's actually an error 
> in the StAX test where the handler was missed. I filed a new bug:
> https://bugs.openjdk.java.net/browse/JDK-8166398
>
>>
>> In particular would it be OK to encounter both CHARACTERS and
>> ENTITY_REFERENCE, or are these exclusive. If they are should some
>> exception be thrown?
>
> They can't. At any given point, the StAX parser returns an unique 
> event and its event type.
>>
>> I can't say I understand how the new test works, but nothing
>> else jumped at me in your webrev :-)
>
> The incorrect javadoc was enough to raise an alarm, that this part of 
> code was copied and forgot to update. It led me to double-check the 
> code and found the issue (JDK-8166398). I really appreciate it!
>
> Best,
> Joe
>>
>> best regards,
>>
>> -- daniel
>>
>>
>> On 19/09/16 20:11, Joe Wang wrote:
>>> Hi,
>>>
>>> Please review an addition of StAX tests to the Catalog Support test 
>>> set.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8166220
>>> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8166220/webrev/
>>>
>>> Thanks,
>>> Joe
>>>
>>