On Tue, 10 Mar 2026 11:38:34 GMT, David Beaumont <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update remaining assertEquals() manually
>
> test/jaxp/javax/xml/jaxp/functional/javax/xml/datatype/ptests/FactoryNewInstanceTest.java
> line 97:
>
>> 95: */
>> 96: @ParameterizedTest
>> 97: @JAXPDataProvider.NewInstanceNeg
>
> This new `@JAXPDataProvider.NewInstanceNeg` annotation is sadly very
> necessary. Without it, or something similar, you need to use
> `@MethodSource("class-name#method")` but that doesn't actually cause the
> compiler to bring in the class in question (`JAXPDataProvider`) so it fails
> at runtime. Defining a new JUnit data provider annotation means the target
> class is in scope at compilation time.
>
> Overall the whole thing of having an opaque shared class for exactly one bit
> of trivial test data is a complexity I'd love to remove altogether, but this
> is the least invasive thing I can do for now.
This is not necessary. You just need to add `@build
jaxp.library.JAXPDataProvider`.
> test/jaxp/javax/xml/jaxp/functional/javax/xml/parsers/ptests/DBFNamespaceTest.java
> line 85:
>
>> 83: Document doc = dbf.newDocumentBuilder().parse(new File(XML_DIR,
>> "namespace1.xml"));
>> 84: dummyTransform(doc, outputfile);
>> 85: assertTrue(compareWithGold(goldfile, outputfile));
>
> compareWithGold() is a util method in JAXPTestUtilities, but that cannot be
> compiled for JUnit tests (it used TestNG classes). Luckily JUnit has a
> good-enough assert for making this a one-line replacement.
There is also `jdk.test.lib.Asserts#assertFileContentsEqual(Path,Path)`.
> test/jaxp/javax/xml/jaxp/functional/javax/xml/stream/ptests/XMLInputFactoryNewInstanceTest.java
> line 83:
>
>> 81: @MethodSource("getValidateParameters")
>> 82: public void testNewFactory(String factoryId, ClassLoader
>> classLoader) {
>> 83: System.setProperty(XMLINPUT_FACTORY_ID,
>> XMLINPUT_FACTORY_CLASSNAME);
>
> Tests use a mix of techniques to handle system properties (try-finally or
> test life-cycle methods). I'd be happy to make everything follow the same
> pattern if you (my dear reviewer) want.
I'm in favor of keeping the diff as focused as possible, i.e., only
TestNG-to-JUnit migration changes. IMHO, improvements/changes outside of this
scope should be addressed separately.
> so I'd be happy to just remove them.
That'd be of my preference too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2931669908
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2931252857
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2931369926
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2931496270
