On Thu, 22 Jan 2026 12:14:05 GMT, Alan Bateman <[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 > > test/jdk/java/sql/driverModuleTests/DriverManagerModuleTests.java line 45: > >> 43: * via the service-provider loading mechanism. >> 44: */ >> 45: @TestInstance(TestInstance.Lifecycle.PER_CLASS) > > Has migration replaces a data provider with a method source that is an > instance method, when JUnit prefers a static method? I'm trying to understand > why Lifecycle.PER_CLASS is needed. It is my understanding that the migration tool is performing the migration in a way that makes review as easy as possible and follows https://github.com/junit-team/junit-framework/wiki/Migrating-from-TestNG-to-JUnit-Jupiter. So adjusting the JUnit test to launch an instance per class to match the TestNG semantics allows us to make the code diff smaller and the conversion easier (w/o any possible behavioral changes). However, I agree that if writing these JUnit tests from scratch, we would use the default life cycle. I'll update these tests to use per method semantics, but this might be worth bringing up in discussion with the others involved in the JUnit conversions. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29354#discussion_r2722198387
