On Wed, 11 Mar 2026 15:31:18 GMT, David Beaumont <[email protected]> wrote:
> Refactor remaining tests in test/jaxp/javax/xml/jaxp/functional to use junit, > along with ancillary utilities and a handful of related tests elsewhere. > > The difficulty in these refactorings is the use of common utilities which > themselves depend on TestNG classes, which are not available when running > JUnit tests. Thus, several bits of functionality in utility classes (esp. > classes in jaxp.library) has had to be re-implemented and inlined. This isn't > terrible, since most of these were one line functions (or complex functions > which could be replaced with one line). > > The only exception to this are the handful of tests using > `compareDocumentWithGold` which is too large to justify inlining. > These tests can be converted at the end when the utility class itself can be > refactored. > > Another complexity is accounting for the differences in test lifecycle > management between TestNG and JUnit. A few classes needed to exploit the > lifecycle and execution modes for single setup and single threaded operation. This PR says it's the "remaining" tests, but as of now https://github.com/openjdk/jdk/pull/30165 hasn't gone in yet. It will be the remaining tests (or at least almost all of them) once that's in. test/jaxp/javax/xml/jaxp/functional/org/w3c/dom/ptests/ElementTest.java line 236: > 234: public void testToString() throws Exception { > 235: final String xml = > 236: "<?xml version=\"1.0\" encoding=\"UTF-8\" > standalone=\"yes\"?>" I know it's not directly related, but the text here isn't actually split between lines (no trailing '\n') and text blocks are just easier to read. test/jaxp/javax/xml/jaxp/functional/test/astro/DocumentLSTest.java line 87: > 85: src.setCertifiedText(origCertified); // set back to orig > 86: > 87: src.setSystemId(filenameToURL(ASTROCAT)); This little util method is used in a few dozen places, and while it's simple enough to inline (to break the dependency on JAXPTestUtilities, which is a required step) I do want to come back later and look at sorting this out. A big part would be making all the directory constants in Path rather than String which just shortens the expression needed. test/jaxp/javax/xml/jaxp/functional/test/auctionportal/AuctionController.java line 364: > 362: > 363: /** Convert file contents to a given character set with BOM marker. > */ > 364: public static InputStream utf16Stream(String file, ByteOrder > byteOrder) This is a simpler version of `bomStream` which cannot be used in JUnit test classes because it exists in JAXPTestUtilities, which cannot be used due to NestNG dependencies. This version is actually better because it lets us test with both possible byte orders, rather than only the native one. test/jaxp/javax/xml/jaxp/libs/test/auctionportal/HiBidConstants.java line 34: > 32: * XML source file directory. > 33: */ > 34: public static final String XML_DIR = > getPathByClassName(HiBidConstants.class, "content"); Once all the tests are done I want to convert all these constants to Path, since the need for explicit addition of a trailing slash on all these String is quite fragile. I'd also investigate a common constants/util class to avoid so much repetition. ------------- PR Review: https://git.openjdk.org/jdk/pull/30201#pullrequestreview-3930534002 PR Review Comment: https://git.openjdk.org/jdk/pull/30201#discussion_r2919207310 PR Review Comment: https://git.openjdk.org/jdk/pull/30201#discussion_r2919314647 PR Review Comment: https://git.openjdk.org/jdk/pull/30201#discussion_r2919345722 PR Review Comment: https://git.openjdk.org/jdk/pull/30201#discussion_r2919293487
