On Thu, 22 Jan 2026 00:41:13 GMT, Justin Lu <[email protected]> wrote:

> Please review this PR which converts the JDBC TestNG tests to use JUnit.
> 
> This is mainly done using the automated tool in 
> https://github.com/openjdk/jdk/commit/0cec3097aec02e72ccb6ebbf0b2b046220578d1b,
>  with some manual follow up commits. The testng folder is migrated to junit 
> with the TEST.properties updated as well. Most changes are annotation updates 
> and switching from testNG imports to JUnit. I decided to simplify cases of 
> `BaseTest.trueFalse()` to use booleans in a `ValueSource` directly in 
> https://github.com/openjdk/jdk/commit/757e7966666d39748db2912b32ccf8b1df18bd62.
> 
> Framework test stats before:
> 680 = 680 TestNG + 0 JUnit
> 
> Framework test stats after:
> 680 = 0 TestNG + 680 JUnit

Hi Justin

Thank you for taking this on.

As part of this PR, we should also tackle javax/sql as they refer back to some 
of the code in this area.


We probably can look to convert the JavaTimeTest.java to junit and then we can 
up level from the junit, formally testing, folder.

A couple of other quick comments below

test/jdk/java/sql/junit/test/sql/CallableStatementTests.java line 135:

> 133:     @Test
> 134:     public void test08() throws SQLException {
> 135:         Assertions.assertThrows(NullPointerException.class, () -> 
> cstmt.enquoteNCharLiteral(null));

Maybe use an import so we can shorten to just assertThrow?

test/jdk/java/sql/junit/util/BaseTest.java line 80:

> 78:      * DataProvider used to specify the standard JDBC Types
> 79:      */
> 80:     protected Object[][] jdbcTypes() {

Suggest converting all of these methods to use Stream<Arguments> and return 
Stream.of.

-------------

PR Review: https://git.openjdk.org/jdk/pull/29354#pullrequestreview-3692915553
PR Review Comment: https://git.openjdk.org/jdk/pull/29354#discussion_r2717273775
PR Review Comment: https://git.openjdk.org/jdk/pull/29354#discussion_r2717981622

Reply via email to