On Wed, 14 Jan 2026 00:12:09 GMT, Justin Lu <[email protected]> wrote:
>> This PR converts the TestNG tests in _util/ServiceLoader_ (7),
>> _util/StringJoiner_ (3), and _util/Vector_ (1) to JUnit.
>> The first commit is from the automated tool, the rest are manual changes
>> done after.
>>
>> Before
>> ServiceLoader: Framework-based tests: 62 = 62 TestNG + 0 JUnit
>> StringJoiner: Framework-based tests: 32 = 32 TestNG + 0 JUnit
>> Vector: Framework-based tests: 9 = 9 TestNG + 0 JUnit
>>
>> After
>> ServiceLoader: Framework-based tests: 62 = 0 TestNG + 62 JUnit
>> StringJoiner: Framework-based tests: 32 = 0 TestNG + 32 JUnit
>> Vector: Framework-based tests: 9 = 0 TestNG + 9 JUnit
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Retain the removed whitespace in BadProvidersTest.java
test/jdk/java/util/ServiceLoader/BadProvidersTest.java line 142:
> 140:
> 141:
> 142: public static Object[][] createBadFactories() {
I assume the method source doesn't need to be public. Also it could be changed
to return `Stream<List>` and that would allow the "ignore" parameter to be
dropped.
test/jdk/java/util/ServiceLoader/BadProvidersTest.java line 173:
> 171: // load providers and instantiate each one
> 172: loadProviders(mods, TEST1_MODULE).forEach(Provider::get);
> 173: });
The scope of the function passed to assertThrows is way too large. The code to
compile the factory and copy the compiled class into the module should not be
in this block. Was this tool or manual edit? Asking because we should only
assert that the SL use throws ServiceConfigurationError.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29210#discussion_r2689298479
PR Review Comment: https://git.openjdk.org/jdk/pull/29210#discussion_r2689295671
