[FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst by spuru9 · Pull Request #28261 · apache/flink

spuru9 · 2026-05-26T14:59:09Z

What is the purpose of the change

Migrate all JUnit 4 tests in flink-state-backends/flink-statebackend-forst to JUnit 5 (Jupiter) as part of the ongoing migration tracked under FLINK-25325. This eliminates the last JUnit 4 imports from the ForSt module.

JIRA: FLINK-39767

Brief change log

9 test files migrated:

ForStIncrementalCheckpointRescalingTest — parameterized: @RunWith(Parameterized.class) → @ExtendWith(ParameterizedTestExtension.class) with @TestTemplate
ForStInitITCase
ForStMemoryControllerUtilsTest
ForStMultiClassLoaderTest
ForStNativeMetricOptionsTest
ForStOperationsUtilsTest
ForStResourceContainerTest
ForStStateBackendConfigTest
ForStStateBackendFactoryTest

Per-file changes:

org.junit.* imports → org.junit.jupiter.api.*
@Before / @After / @BeforeClass / @AfterClass → @BeforeEach / @AfterEach / @BeforeAll / @AfterAll
@Rule TemporaryFolder / @ClassRule TemporaryFolder → @TempDir java.nio.file.Path + TempDirUtils.newFolder(...) / TempDirUtils.newFile(...)
@Test(expected = X.class) → assertThatThrownBy(...).isInstanceOf(X.class)
Hamcrest + org.junit.Assert.* → AssertJ (project convention)
Drop extends TestLogger and public modifiers per the Flink JUnit 5 Migration Guide

No production code is touched.

Verifying this change

This change is a test refactor and is covered by the migrated tests themselves:

./mvnw -pl flink-state-backends/flink-statebackend-forst test

Result: 976 tests run, 0 failures, 0 errors, 5 skipped. Total test count and pass/skip distribution match 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

@afterclass

Migrate all JUnit 4 tests in flink-statebackend-forst to JUnit 5 (Jupiter): - Replace org.junit.* imports with org.junit.jupiter.api.* - @Before/@After/@BeforeClass/@afterclass -> @BeforeEach/@AfterEach/@BeforeAll/@afterall - @Rule/@ClassRule TemporaryFolder -> @tempdir + TempDirUtils helpers - @test(expected = X.class) -> assertThatThrownBy(...).isInstanceOf(X.class) - @RunWith(Parameterized.class) -> @ExtendWith(ParameterizedTestExtension.class) with Flink's @Parameters/@parameter annotations and @testtemplate - Hamcrest + org.junit.Assert.* assertions -> AssertJ (project convention) - Drop extends TestLogger and public modifiers per the migration guide

flinkbot · 2026-05-26T15:10:59Z

CI report:

5b83cfa Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

@test

… tests For ForSt tests that only need one fresh temp dir per method, switch from the TempDirUtils helper + field to native JUnit 5 @tempdir parameter injection: - ForStInitITCase.testTempLibFolderDeletedOnFail(@tempdir File tempFolder) - ForStMemoryControllerUtilsTest.ensureRocksDbNativeLibraryLoaded(@tempdir Path) - ForStMultiClassLoaderTest.testTwoSeparateClassLoaders(@tempdir Path tmp) - ForStOperationsUtilsTest @BeforeAll/@test both take own @tempdir param The remaining files (ForStResourceContainerTest, ForStStateBackendConfigTest, ForStStateBackendFactoryTest, ForStIncrementalCheckpointRescalingTest) keep TempDirUtils because they create many subdirs from one parent root, which is the documented use case for the helper per the Flink JUnit 5 migration guide.

…mentalCheckpointRescalingTest The file doesn't import org.apache.flink.core.fs.Path so the FQN 'java.nio.file.Path' wasn't needed; use a normal import instead.

spuru9 · 2026-05-29T13:29:37Z

@snuyanzin Can you review this as well.

snuyanzin · 2026-06-01T07:00:06Z

 * with the License.  You may obtain a copy of the License at
 *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0


why do we need it?
Did not spotless catch it?

snuyanzin · 2026-06-01T07:00:30Z

+        assertThat(
+                        KeyGroupRangeAssignment.assignToKeyGroup(
+                                keySelector.getKey(records[0]), maxParallelism))
+                .isEqualTo(0); // group 0


Suggested change

.isEqualTo(0); // group 0

.isZero(); // group 0

snuyanzin · 2026-06-01T07:00:43Z

+        assertThat(
+                        KeyGroupRangeAssignment.assignToKeyGroup(
+                                keySelector.getKey(records[1]), maxParallelism))
+                .isEqualTo(1); // group 1


Suggested change

.isEqualTo(1); // group 1

.isOne(); // group 1

snuyanzin · 2026-06-01T07:01:13Z

            harness.setCheckpointStorage(
                    new FileSystemCheckpointStorage(
-                            "file://" + rootFolder.newFolder().getAbsolutePath()));
+                            "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));


why not @TempDir?

Have used @TempDir now in the places only one directory is needed. But the Utils in the case of multiple, I believe its a cleaner approach to not use multiple @TempDir, also TempDirUtils is created for the same purpose.

I was using TempDirUtils.newFolder(rootFolder).getAbsolutePath())) as it is being recommended in https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit?tab=t.0#heading=h.we2n5aifcjfq

We provide a util class TempDirUtils for implementing common functions in TemporaryFolder, like newFolder() and newFile().

snuyanzin · 2026-06-01T07:01:33Z

            harness2[0].setCheckpointStorage(
                    new FileSystemCheckpointStorage(
-                            "file://" + rootFolder.newFolder().getAbsolutePath()));
+                            "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));


@TempDir?

snuyanzin · 2026-06-01T07:01:45Z

            harness2[1].setCheckpointStorage(
                    new FileSystemCheckpointStorage(
-                            "file://" + rootFolder.newFolder().getAbsolutePath()));
+                            "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));


@TempDir?
here and in other places

snuyanzin · 2026-06-01T07:04:16Z

+        assertThat(pathsArray).isNotNull();
+        assertThat(paths).isNotNull();
+        assertThat(pathsArray).hasSize(paths.length);
+        assertThat(pathsArray).contains(paths);


how about containsExactlyInAnyOrder?
then no need to check for size

- ForStIncrementalCheckpointRescalingTest: use AssertJ isZero()/isOne() for the group-0/group-1 key-group assertions; revert the unrelated license-header indentation change back to master. - ForStStateBackendFactoryTest: collapse hasSize(...) + contains(...) in checkPaths into a single containsExactlyInAnyOrder(...).

…mentalCheckpointRescalingTest Per review: the per-harness TempDirUtils.newFolder(rootFolder) calls are not needed for isolation. setCheckpointStorage() creates the checkpoint storage with a fresh JobID per call, so each harness's checkpoints are already namespaced under <base>/<jobId>/ even when the base directory is shared. Point all harnesses at the injected @tempdir and drop the unused TempDirUtils import.

spuru9 · 2026-06-01T08:48:03Z

-        final String localDir3 = tmp.newFolder().getAbsolutePath();
-        final String localDir4 = tmp.newFolder().getAbsolutePath();
+    void testLoadForStStateBackendMixed() throws Exception {
+        final String localDir1 = TempDirUtils.newFolder(tmp).getAbsolutePath();


Kept TempDirUtils.newFolder(tmp) since the paths are used as strings and one test needs four — tidier than multiple @tempdir params; distinctness is needed either way.

… ForStResourceContainerTest The native-library load needs only one directory, so inject it as a @tempdir parameter (matching ForStOperationsUtilsTest) instead of TempDirUtils.newFolder(tmpFolder). The static tmpFolder field and the helper remain for testDirectoryResources/testFileSystemInit, which each need multiple distinct directories.

spuru9 · 2026-06-01T09:38:29Z

-    public void testDirectoryResources() throws Exception {
-        Path localJobPath = new Path(TMP_FOLDER.newFolder().getPath());
+    void testDirectoryResources() throws Exception {
+        Path localJobPath = new Path(TempDirUtils.newFolder(tmpFolder).getPath());


Multiple distinct temp dirs are needed here, so TempDirUtils.newFolder(tmpFolder) is simpler than injecting several @TempDir params.

spuru9 marked this pull request as draft May 28, 2026 04:08

spuru9 added 2 commits May 28, 2026 11:01

[FLINK-39767][tests] Drop unneeded fully-qualified Path in ForStIncre…

737267f

…mentalCheckpointRescalingTest The file doesn't import org.apache.flink.core.fs.Path so the FQN 'java.nio.file.Path' wasn't needed; use a normal import instead.

spuru9 marked this pull request as ready for review May 28, 2026 06:31

Empty commit to retrigger CI

caa3cbe

snuyanzin reviewed Jun 1, 2026

View reviewed changes

spuru9 added 2 commits June 1, 2026 12:52

spuru9 commented Jun 1, 2026

View reviewed changes

github-actions Bot added the community-reviewed PR has been reviewed by the community. label Jun 1, 2026

spuru9 requested a review from snuyanzin June 1, 2026 10:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst#28261

[FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst#28261
spuru9 wants to merge 7 commits into
apache:masterfrom
spuru9:feature/junit5-forst

spuru9 commented May 26, 2026

Uh oh!

flinkbot commented May 26, 2026 •

edited

Loading

Uh oh!

spuru9 commented May 29, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

spuru9 Jun 1, 2026 •

edited

Loading

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

snuyanzin Jun 1, 2026

Uh oh!

spuru9 Jun 1, 2026

Uh oh!

spuru9 Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

spuru9 commented May 26, 2026

What is the purpose of the change

Brief change log

Verifying this change

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

Documentation

Was generative AI tooling used to co-author this PR?

Uh oh!

flinkbot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

spuru9 commented May 29, 2026

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!

spuru9 Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

flinkbot commented May 26, 2026 •

edited

Loading

spuru9 Jun 1, 2026 •

edited

Loading