Migrate tests to `@ParameterizedClass` in `spring-core` by CodingMasterLSW · Pull Request #36181 · spring-projects/spring-framework

CodingMasterLSW · 2026-01-20T12:19:35Z

This PR migrates parameterized tests to @ParameterizedClass in the spring-core module.

To prioritize code cohesion, I actively use @Nested test classes.

See Migrate to @ParameterizedClass where feasible #35833

sbrannen · 2026-01-20T15:03:23Z

Please provide the DCO sign-off in all commits.

Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

CodingMasterLSW · 2026-01-20T15:22:43Z

@sbrannen
I have updated the commits to include the DCO sign-off. Thanks for pointing that out!

sbrannen

Thanks for the first PR in this "series"! 👍

I think we'll "learn as we go" regarding how best to achieve this migration.

So, I've requested a few changes as a first step.

sbrannen · 2026-01-21T10:59:22Z

...-core/src/test/java/org/springframework/core/io/buffer/support/DataBufferTestUtilsTests.java

+@ParameterizedClass
+@MethodSource("org.springframework.core.testfixture.io.buffer.AbstractDataBufferAllocatingTests#dataBufferFactories()")


Can't this be declared directly on AbstractDataBufferAllocatingTests?

I moved these annotations to the AbstractDataBufferAllocatingTests class.

sbrannen · 2026-01-21T11:00:06Z

...-core/src/test/java/org/springframework/core/io/buffer/support/DataBufferTestUtilsTests.java

+	DataBufferTestUtilsTests(DataBufferFactory bufferFactory) {
+		DataBufferTestUtilsTests.this.bufferFactory = bufferFactory;
+	}


Similarly, instead of declaring such constructors in each subclass, I think it would be much better to annotate the bufferFactory field in AbstractDataBufferAllocatingTests with @Parameter.

Oh, That's great point!
I'm new to @parameter, so I appreciate you pointing that out!

sbrannen · 2026-01-21T11:02:48Z

spring-core/src/test/java/org/springframework/core/io/buffer/DataBufferUtilsTests.java

+	@Nested
+	@ParameterizedClass
+	@MethodSource("org.springframework.core.testfixture.io.buffer.AbstractDataBufferAllocatingTests#dataBufferFactories()")
+	class ParameterizedDataBufferUtilsTests {


I think the @Nested test classes like this one should extend AbstractDataBufferAllocatingTests, and the enclosing classes should no longer extend that.

That makes sense to me.
Reflected your feedback in 672157b.

sbrannen · 2026-01-21T12:07:19Z

spring-core/src/test/java/org/springframework/core/io/ResourceTests.java

+			assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() -> relative4.getContentAsString(UTF_8));
+		}
+
+		private static Stream<Arguments> resource() throws URISyntaxException {


Suggested change

private static Stream<Arguments> resource() throws URISyntaxException {

private static Stream<Arguments> resources() throws URISyntaxException {

While we're at it, we might as well rename that method to "resources".

Reflected your feedback in 84d677b

sbrannen · 2026-01-21T12:09:36Z

spring-core/src/test/java/org/springframework/util/MultiValueMapTests.java

+		private final MultiValueMap<String, String> map;
+
+		public ParameterizedMultiValueMapTests(MultiValueMap<String, String> map) {
+			this.map = new LinkedMultiValueMap<>(map);


Suggested change

this.map = new LinkedMultiValueMap<>(map);

this.map = map;

I found a potential issue with using @ParameterizedClass in this case.

Since the parameter object from @MethodSource is created only once and shared across all test methods,
modifying it (e.g., this.map = map;) causes state to leak between tests.

To ensure test isolation, I've considered the following options:

Create a defensive copy in the constructor (current approach)
Since the constructor is called for each test method, instantiating a new object ensures each test gets its own isolated instance.

Use @ParameterizedTest instead
Each test method receives a fresh parameter instance, but this requires repeating the parameter declaration for every test method.

Please let me know if there's a better way to handle this!

…ferAllocatingTests Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

CodingMasterLSW · 2026-01-21T16:14:55Z

Hi @sbrannen,

I've applied your feedback. Additionally, I’ve updated ServerSentEventHttpMessageWriterTests and WebClientDataBufferAllocatingTests (which inherit from AbstractDataBufferAllocatingTests) to use the @test annotation as well.

Also, I’ve left a comment regarding a potential issue with one of the suggested changes. I’d appreciate it if you could share your thoughts on that.

Thanks!

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 20, 2026

CodingMasterLSW mentioned this pull request Jan 20, 2026

Migrate to @ParameterizedClass where feasible #35833

Open

sbrannen changed the title ~~Migrate ResourceTests to @ParameterizedClass in spring-core~~ Migrate tests to @ParameterizedClass in spring-core Jan 20, 2026

sbrannen added the type: task A general task label Jan 20, 2026

sbrannen self-assigned this Jan 20, 2026

CodingMasterLSW added 2 commits January 21, 2026 00:17

fix: Migrate tests to @ParameterizedClass in spring-core

95918fd

Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

fix: Add blank line between JUnit and Spring imports

151df60

Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

CodingMasterLSW force-pushed the fix-parameterized-class branch from f8245c7 to 151df60 Compare January 20, 2026 15:19

sbrannen reviewed Jan 21, 2026

View reviewed changes

CodingMasterLSW added 2 commits January 22, 2026 00:37

fix: Centralize @ParameterizedClass, @MethodSource in AbstractDataBuf…

672157b

…ferAllocatingTests Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

fix: Rename resource() to resources() in ResourceTests

84d677b

Signed-off-by: CodingMasterLSW <spqjekdl1004@gmail.com>

		@ParameterizedClass
		@MethodSource("org.springframework.core.testfixture.io.buffer.AbstractDataBufferAllocatingTests#dataBufferFactories()")

	private static Stream<Arguments> resource() throws URISyntaxException {
	private static Stream<Arguments> resources() throws URISyntaxException {

Conversation

CodingMasterLSW commented Jan 20, 2026 • edited by sbrannen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbrannen commented Jan 20, 2026

Uh oh!

CodingMasterLSW commented Jan 20, 2026

Uh oh!

sbrannen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CodingMasterLSW commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CodingMasterLSW commented Jan 20, 2026 •

edited by sbrannen

Loading