Skip to content

[FLINK-39770][tests][JUnit5 migration] Module: flink-state-processing-api#28561

Open
spuru9 wants to merge 1 commit into
apache:masterfrom
spuru9:feature/junit5-state-processing-api
Open

[FLINK-39770][tests][JUnit5 migration] Module: flink-state-processing-api#28561
spuru9 wants to merge 1 commit into
apache:masterfrom
spuru9:feature/junit5-state-processing-api

Conversation

@spuru9

@spuru9 spuru9 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Migrate the remaining JUnit 4 tests in flink-libraries/flink-state-processing-api to JUnit 5 (Jupiter). After this PR the module has zero JUnit 4 imports. Part of the umbrella JUnit 4 → 5 migration.

JIRA: FLINK-39770 (sub-task of FLINK-25325)

Brief change log

20 test classes migrated across the module (org.apache.flink.state.api and its input / output / runtime / utils sub-packages):

  • org.junit.* imports → org.junit.jupiter.api.*; lifecycle annotations @Before*/@After*@BeforeEach/@AfterEach/@BeforeAll/@AfterAll.
  • extends AbstractTestBaseJUnit4extends AbstractTestBase (the JUnit 5 sibling) for the six ITCase / base classes.
  • Two parameterized tests (SavepointDeepCopyTest, SavepointWriterWindowITCase): @RunWith(Parameterized.class)@ExtendWith(ParameterizedTestExtension.class) with @Parameter field injection and @TestTemplate.
  • @Rule TemporaryFolder@TempDir for single-dir cases; TempDirUtils retained where a test needs several distinct dirs.
  • @Test(expected = X.class)assertThatThrownBy(...).isInstanceOf(X.class) (with message assertions where the original test checked them).
  • Hamcrest + org.junit.Assert.* → AssertJ; drop public modifiers on test classes/methods per the Flink JUnit 5 Migration Guide.

No production code is touched.

Verifying this change

This change is a test-only migration covered by the existing tests it migrates:

./mvnw -pl flink-libraries/flink-state-processing-api verify

Result: 115 tests run (53 unit + 62 integration), 0 failures, 0 errors, 0 skipped, matching the pre-migration baseline.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code

@flinkbot

flinkbot commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@spuru9 spuru9 force-pushed the feature/junit5-state-processing-api branch from f7b4dfc to eded373 Compare June 27, 2026 17:00
@@ -67,7 +63,6 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@spuru9 spuru9 Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JUnit 4's Parameterized runner requires a @Parameters factory method and @Parameter fields. This class has neither, so it was rdundant.

@spuru9 spuru9 force-pushed the feature/junit5-state-processing-api branch 3 times, most recently from ed787bc to 6059cdf Compare June 27, 2026 18:09
import org.apache.flink.streaming.api.operators.OneInputStreamOperator;
import org.apache.flink.streaming.api.operators.StreamMap;
import org.apache.flink.streaming.util.KeyedOneInputStreamOperatorTestHarness;
import org.apache.flink.testutils.junit.utils.TempDirUtils;

@spuru9 spuru9 Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used because give a fresh dir on each call to a helper getHarness (is called multiple times).
In all places in this file to maintain consistency.

@spuru9 spuru9 marked this pull request as ready for review June 27, 2026 18:49
@spuru9

spuru9 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@snuyanzin Can you review this?
Add a few justification for 1-2 changes.


Assert.assertEquals("Unexpected number of keys", 2, keys.size());
Assert.assertEquals("Unexpected keys found", Arrays.asList(1, 2), keys);
assertThat(keys).isEqualTo(Arrays.asList(1, 2));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is messaged removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the JUnit-style assertion messages since AssertJ's failure output (expected … but was …) plus the test/method name already convey them.
Happy to preserve if you need the, I flt thy were redundant is most case. Or if you want it handled on case to case basis.


Assert.assertEquals("Unexpected number of keys", 2, keys.size());
Assert.assertEquals("Unexpected keys found", Arrays.asList(1, 2), keys);
assertThat(keys).isEqualTo(Arrays.asList(1, 2));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is java 11, use List.of

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jun 28, 2026
@spuru9 spuru9 force-pushed the feature/junit5-state-processing-api branch from 6059cdf to 6c997e0 Compare June 28, 2026 03:30
@spuru9 spuru9 requested a review from snuyanzin June 28, 2026 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants