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

[Joe Wang]

Thanks Daniel!

I'm glad the issue was caught! I'm also making sure any the debugging 
method shall throw Exception as well :-)

Best,
Joe

On 9/22/16, 2:09 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> Looks good to me.
>
> Thanks for having taken the time to take a second look :-)
>
> best regards,
>
> -- daniel
>
> On 21/09/16 17:47, Joe Wang wrote:
>> 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
>>>>>
>>>>
>