On Wed, 14 Jan 2026 17:58:36 GMT, Justin Lu <[email protected]> wrote:
>> 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. > > This is done by the tool, presumably because when testNG uses the > `expectedExceptions` attribute, it permits the exception to be thrown > _anywhere_ in the test, so the conversion tool is guranteeing that the JUnit > replacement has the _exact_ same behavior. I agree we should narrow down the > scope though. Okay, maybe not a good use of expectedExceptions because the scope is entire method. Your update looks good. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29210#discussion_r2691772851
