On Sun, 6 Dec 2020 00:00:24 GMT, Michael Edgar
<github.com+20868526+mikeed...@openjdk.org> wrote:
>> Allow `XMLStreamException` to be thrown to the application when a filtered
>> `XMLStreamReader` encounters an `XMLStreamException` advancing the wrapped
>> `XMLStreamReader`.
>>
>> Note, this PR includes a method signature change (constructor of
>> `com.sun.org.apache.xerces.internal.impl.XMLStreamFilterImpl`).
>
> Michael Edgar has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains two commits:
>
> - Update test for jtreg
> - 8255918: Throw XMLStreamExceptions from XMLStreamFilterImpl constructor
I left comments the other day but forgot to submit. I've adjusted them based on
your new changes.
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamFilterImpl.java
line 68:
> 66: * the filter to apply to the reader
> 67: * @throws XMLStreamException
> 68: * when an <code>XMLStreamException</code> is thrown when
{@code XMLStreamException} is preferable.
test/jdk/javax/xml/jaxp/parsers/8255918/XMLStreamReaderFilterTest.java line 1:
> 1: /*
Please move the test to
test/jaxp/javax/xml/jaxp/unittest/stream/XMLStreamFilterTest
Once this is done, run tier2 test instead of tier1.
test/jdk/javax/xml/jaxp/parsers/8255918/XMLStreamReaderFilterTest.java line 38:
> 36:
> 37: static final String XMLSOURCE1 = "<root>\n"
> 38: + " <element1>\n"
Change to sth like the following:
/*
* @test
* @bug 8255918
* @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
* @run testng stream.XMLStreamFilterTest.XMLStreamReaderFilterTest
* @summary (general description is fine as future tests can be added, but add
javadoc to the test itself)
*/
test/jdk/javax/xml/jaxp/parsers/8255918/XMLStreamReaderFilterTest.java line 22:
> 20: * or visit www.oracle.com if you need additional information or have any
> 21: * questions.
> 22: */
Add package stream.XMLStreamFilterTest;
test/jdk/javax/xml/jaxp/parsers/8255918/XMLStreamReaderFilterTest.java line 75:
> 73:
> 74: assertTrue(thrown instanceof XMLStreamException, "Missing or
> unexpected exception type: " + String.valueOf(thrown));
> 75: assertEquals(EXPECTED_MESSAGE1, thrown.getMessage(), "Unexpected
> exception message: " + thrown.getMessage());
It would be good/sufficient to verify XMLStreamException with assertThrows.
Checking the message details can be fragile.
test/jdk/javax/xml/jaxp/parsers/8255918/XMLStreamReaderFilterTest.java line 58:
> 56: * @modules java.xml
> 57: * @run testng/othervm XMLStreamReaderFilterTest
> 58: *
Tags are added in the class description. Change this to a TestCase javadoc or
note.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1209
