On Fri, 13 Mar 2026 13:41:14 GMT, Volkan Yazici <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> tidy up static imports for assertions
>
> test/jaxp/javax/xml/jaxp/functional/javax/xml/datatype/ptests/XMLGregorianCalendarTest.java
> line 106:
>
>> 104: // expected success
>> 105:
>> 106: assertEquals(calendar2.getMillisecond(), ms);
>
> Needs a swap?
The latest push should be correct for all `assertEquals()` calls, but I'll
check the other variants you mention above (I don't recall any use of
`assertSame()` etc. but `assertArrayEquals()` might be around).
> test/jaxp/javax/xml/jaxp/functional/javax/xml/parsers/ptests/SAXParserTest.java
> line 153:
>
>> 151: @MethodSource("getParser")
>> 152: public void testParse09(SAXParser saxparser) {
>> 153: assertThrows(IOException.class, () ->
>> saxparser.parse("no-such-file", DEFAULT_HANDLER));
>
> So `SAXException` in `expectedExceptions` was superfluous?
Yes, I checked this and it only throws IOException. I've very wary about
expecting one or other exception, because that implies non-deterministic code.
> test/jaxp/javax/xml/jaxp/functional/javax/xml/transform/ptests/URIResolverTest.java
> line 52:
>
>> 50: * @test
>> 51: * @library /javax/xml/jaxp/libs
>> 52: * @run junit/othervm javax.xml.transform.ptests.URIResolverTest
>
> Since you touch all `@run` lines anyway, you can also amending `@run ...
> <class-name>` with `@run ... ${test.main.class}`.
I've no idea what that is. Do you literally mean the string
`${test.main.class)` ? I've never seen that used in any test I've looked at.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2932585389
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2932595622
PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2932606927
