[elharo]

First one was converted manually but now that we see how it works we can throw the rest into at an LLM.

Converts the dom.dom3.Test class from a main()-driven harness with Assertion.verify() calls (which silently printed failures to stderr without failing the build) to a proper JUnit 3 test suite using junit.framework.TestCase.

• Replaces Assertion.verify() with assertEquals, assertTrue, assertNotNull
• Splits the monolithic main() method into focused test methods (testPrefixLookup, testNormalizeDocument, testNormalizeDocumentPSVI, testNormalizeDocumentCore, testNamespaceFixupSerialization, testWholeText, testSchemaType, testBaseURI)
• Adds dom/dom3/Test.class to the <batchtest> in build.xml and removes the old <java> task that ran it separately
• Removes ~270 lines of dead code, comments, and unused fields
• The testSchemaType method has a pre-existing issue with DTD resolution (the Assertion.verify in the original was never reached due to a catch-all in main()). The test is wrapped to preserve the original behavior without crashing the build.

[elharo]

assertion message is backwards here, and probably not needed at all

[elharo]

in these and similar tests below it's enough to use assertEquals; you don't need to check for null

e.g.

assertEquals("http://child1", value);

[elharo]

add a TODO to fix this

[Copilot]

Pull request overview

This PR converts dom.dom3.Test from a main()-driven harness into a JUnit 3 TestCase, so DOM Level 3 checks run as part of the Ant JUnit suite (and can fail CI appropriately) rather than only printing failures to stderr.

Changes:

• Replaces Assertion.verify()-style checks with JUnit assertions and splits the previous monolithic flow into dedicated test* methods.
• Registers dom/dom3/Test.class in the Ant <batchtest> and removes the standalone <java classname="dom.dom3.Test"> invocation.
• Refactors DOMError handling and LS resource resolution to integrate with the JUnit-based execution model.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File	Description
tests/dom/dom3/Test.java	Converts the DOM Level 3 harness into a JUnit 3 TestCase with multiple focused test methods and in-test error handling/resource resolution.
build.xml	Runs dom.dom3.Test via the JUnit batch suite and removes the separate java task that previously executed it.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lyca]

👍 (non-binding). Splitting the huge main() method into individual tests and swapping the custom assertions to standard JUnit looks very clean! I left a few inline comments regarding proper test isolation and a missed W3C spec detail, but other than that, this looks good to me!

[lyca]

This could be handled with the same pattern you used in #116: saving the property's original value here and restoring/clearing it in a tearDown() method to guarantee test isolation.

[elharo]

done

[lyca]

I just had another look at this. Setting DOMImplementationRegistry.PROPERTY shouldn't be necessary in the tests at all nowadays. The correct fallback is already provided natively via the ServiceLoader pattern.

[lyca]

Suggested change

	} else {
	source.setSystemId("data/personal.dtd");
	} else if (systemId != null && systemId.endsWith("personal.dtd")) {
	source.setSystemId("data/personal.dtd");
	} else {
	return null;
	}

[lyca]

This would fix the TODO on line 330.

[lyca]

According to the DOM Level 3 Core specification, replacing this node is strictly expected to fail and raise a NO_MODIFICATION_ALLOWED_ERR. Because entities=true is set earlier, the logically adjacent EntityReference node (&ent5;) is expanded as read-only. We should likely revert this specific check to a try-catch asserting the DOMException, otherwise the test loses its ability to enforce the W3C spec compliance here. -> https://www.w3.org/TR/DOM-Level-3-Core/core.html#Text3-replaceWholeText

[elharo]

done

[elharo]

except it doesn't fail. This might be a bug.

[lyca]

You're right, this seems to be an edge case with the nested entity this test wanted to cover.

Why Xerces fails to throw it (TextImpl.java): To validate the replacement, TextImpl calls canModifyNext(Node). When it encounters a nested entity, it recursively calls canModifyNext:

1. canModifyNext("Test: ")
2. Iterates to adjacent entity &ent5;
3. Iterates over children of &ent5; and hits nested entity &ent6;
4. Recursively calls canModifyNext(&ent6;)
5. The Bug: canModifyNext(node) immediately calls node.getNextSibling(). As a result, it skips checking the children of &ent6;, completely missing the element.