On Tue, 10 Mar 2026 15:27:56 GMT, Sam Brannen <[email protected]> wrote:

>> test/jaxp/javax/xml/jaxp/functional/javax/xml/datatype/ptests/FactoryNewInstanceTest.java
>>  line 54:
>> 
>>> 52:     private static final String DATATYPE_FACTORY_CLASSNAME = 
>>> DEFAULT_IMPL_CLASS;
>>> 53: 
>>> 54:     public static Object[][] getValidateParameters() {
>> 
>> One of the most common changes is making data provider methods static 
>> (required in JUnit). Most data provider method and many fields or one-line 
>> util methods could always have been static.
>> 
>> I've reformatted a few of these just for my own sanity while trying to 
>> reason about what's actually being provided (sometimes it's just a single 
>> variable and can use `@ValueSource` instead of `@MethodSource`.
>
> In case you were not aware, `@MethodSource` factory methods only have to be 
> `static` _by default_.
> 
> If you switch to `@TestInstance(Lifecyle.PER_CLASS)`, they don't have to be 
> `static`, _and_ the lifecycle would align with the semantics of TestNG.
> 
> From the [User 
> Guide](https://docs.junit.org/6.0.3/writing-tests/parameterized-classes-and-tests.html#sources-MethodSource):
> 
>> Factory methods within the test class must be `static` unless the test class 
>> is annotated with `@TestInstance(Lifecycle.PER_CLASS)`; whereas, factory 
>> methods in external classes must always be static.

Indeed. I've done that in a few places, but I don't want to do it by default 
because that will affect the ability to parallelize tests.
Most tests don't use anything mutable or so heavyweight that it would matter if 
it was recreated per test case.
If you think you see any case where I chose wrongly here (e.g. used PER_CLASS 
needlessly) please let me know.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30165#discussion_r2912810240

Reply via email to