RFR: JDK-8264454 : Jaxp unit test from open jdk needs to be improved
Joe Wang
joehw at openjdk.java.net
Wed Mar 31 05:10:30 UTC 2021
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
More information about the core-libs-dev
mailing list