RFR: 8255918: XMLStreamFilterImpl constructor consumes XMLStreamException [v2]

[Joe Wang]

On Sun, 6 Dec 2020 00:00:24 GMT, Michael Edgar <github.com+20868526+MikeEdgar at 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