RFR: JDK-8264454 : Jaxp unit test from open jdk needs to be improved

[Joe Wang]

On Tue, 30 Mar 2021 16:56:57 GMT, Mahendra Chhipa <github.com+34924738+mahendrachhipa at openjdk.org> wrote:

> JDK-8264454 : Jaxp unit test from open jdk needs to be improved

test/jaxp/javax/xml/jaxp/unittest/common/Bug6350682.java line 47:

> 45:     @Test
> 46:     public void testSAXParserFactory() {
> 47:         runWithAllPerm(() -> Thread.currentThread().setContextClassLoader(null));

To address item 4 (the environment is changed), a note, something like "this test runs in othervm" can be added here or to the summary.

test/jaxp/javax/xml/jaxp/unittest/common/Bug6723276Test.java line 45:

> 43: @Listeners({jaxp.library.BasePolicy.class})
> 44: public class Bug6723276Test {
> 45:     private static final String ERR_MSG = "org.apache.xerces.jaxp.SAXParserFactoryImpl not found";

This test currently doesn't verify anything since there's no org.apache.xerces.jaxp.SAXParserFactoryImpl on the classpath. Post JDK 9, such test would require creating a dummy module.

test/jaxp/javax/xml/jaxp/unittest/common/Bug6723276Test.java line 48:

> 46: 
> 47:     @Test
> 48:     public void testSAXParserFactoryCreationWithDefaultContextClassLoader() {

A shorter name such as testWithDefaultClassLoader would be fine.

test/jaxp/javax/xml/jaxp/unittest/common/Bug6723276Test.java line 59:

> 57: 
> 58:     @Test
> 59:     public void testSAXParserFactoryCreationWithGivenURLContextClassLoader() {

A shorter name would be testWithURLClassLoader

test/jaxp/javax/xml/jaxp/unittest/datatype/Bug6320118.java line 59:

> 57:         Assert.assertEquals(calendar.getMonth(), 1);
> 58:         Assert.assertEquals(calendar.getDay(), 2);
> 59:         Assert.assertEquals(calendar.getHour(), 0, "hour 24 needs to be treated as hour 0 of next day");

This change has added assertions, but it seems ok.

test/jaxp/javax/xml/jaxp/unittest/datatype/Bug6937964Test.java line 57:

> 55:      * Constant to indicate expected lexical test failure.
> 56:      */
> 57:     private static final String TEST_VALUE_FAIL = "*FAIL*";

I don't see this in the expected values. It, along with the related assertions, may be removed.

test/jaxp/javax/xml/jaxp/unittest/datatype/Bug6937964Test.java line 194:

> 192:             try {
> 193:                 QName xmlSchemaType = duration.getXMLSchemaType();
> 194:                 if (!xmlSchemaType.equals(DatatypeConstants.DURATION_YEARMONTH)) {

Can be changed to assertEquals

test/jaxp/javax/xml/jaxp/unittest/datatype/Bug6937964Test.java line 200:

> 198:                 } catch (IllegalStateException illegalStateException) {
> 199:                     // TODO; this test really should pass
> 200:                     System.err.println("Please fix this bug that is being ignored, for now: " + illegalStateException.getMessage());

Do we still have such a bug?

test/jaxp/javax/xml/jaxp/unittest/datatype/Bug6937964Test.java line 204:

> 202: 
> 203:                 // does it have the right value?
> 204:                 if (!expectedLex.equals(duration.toString())) {

Can be changed to assertEquals

-------------

PR: https://git.openjdk.java.net/jdk/pull/3274