Skip to content

[FLINK-29775][tests][JUnit5 migration] Module: flink-statebackend-rocksdb - #28943

Open
spuru9 wants to merge 1 commit into
apache:masterfrom
spuru9:FLINK-29775
Open

[FLINK-29775][tests][JUnit5 migration] Module: flink-statebackend-rocksdb#28943
spuru9 wants to merge 1 commit into
apache:masterfrom
spuru9:FLINK-29775

Conversation

@spuru9

@spuru9 spuru9 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Migrates the remaining JUnit 4 tests in flink-statebackend-rocksdb to JUnit 5 and converts their assertions to AssertJ, as part of FLINK-25325.

Most tests in this module have a near-identical sibling in flink-statebackend-forst, which was migrated in FLINK-39767 (6627acb). Where a sibling exists, this PR follows it so the two modules stay consistent.

After this change the module has no reference to org.junit.* (outside org.junit.jupiter), no Hamcrest matchers, and no TestLogger.

Brief change log

  • @Rule / @ClassRule TemporaryFolder@TempDir. A test that needs a single directory takes it as a method parameter (@TempDir File dir); TempDirUtils.newFolder(root) is kept only where one test needs several distinct directories.
  • @RunWith(Parameterized.class)@ExtendWith(ParameterizedTestExtension.class) + @TestTemplate in RocksIncrementalCheckpointRescalingTest, matching ForStIncrementalCheckpointRescalingTest.
  • @Test(expected = ...) and the manual try { ...; fail(); } catch (X e) { ... } blocks → assertThatThrownBy(...) / assertThatCode(...).doesNotThrowAnyException().
  • org.junit.Assert and Hamcrest matchers → AssertJ, preferring the native idioms (hasSize, containsExactly, containsExactlyInAnyOrderElementsOf, isInstanceOf, hasToString, hasBinaryContent, exists/doesNotExist, allMatch, satisfiesAnyOf, hasCause/hasMessageContaining).
  • org.junit.Assumeorg.junit.jupiter.api.Assumptions.
  • Dropped extends TestLogger and the now-unnecessary public modifiers on test classes and methods.
  • RocksDBExtension and RocksDBKeyedStateBackendTestFactory are test utilities rather than tests, but were still holding a JUnit 4 TemporaryFolder. RocksDBExtension (itself a JUnit 5 extension, so it cannot use @TempDir injection) now creates and deletes its own temp directory; RocksDBKeyedStateBackendTestFactory.create(...) takes a java.nio.file.Path instead.

Assertion messages were kept via .as(...). The ones that disappear are fail("...should have thrown")-style messages, which assertThatThrownBy expresses natively, and messages whose content AssertJ already prints (e.g. assertThat(errorRef.get()).isNull() prints the throwable).

Verifying this change

This change is a test-only rework; it is verified by the existing tests it migrates.

mvn verify -pl flink-state-backends/flink-statebackend-rocksdb passes with the same number of tests before and after, per class:

  • before: 843 unit tests (9 skipped) + 2 integration tests
  • after: 843 unit tests (9 skipped) + 2 integration tests

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

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

Generated-by: Claude Code (claude-opus-5)

@flinkbot

flinkbot commented Aug 7, 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 marked this pull request as draft August 7, 2026 17:40
import static org.assertj.core.api.Assertions.assertThat;

/** Test class for {@link DistributeStateHandlerHelper}. */
public class DistributeStateHandlerHelperTest extends TestLogger {

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.

extends TestLoggeris dropped without adding aMETA-INF/servicesfile —TestLoggerExtensionis already auto-registered from theflink-streaming-java` test-jar this module depends on.

@spuru9
spuru9 force-pushed the FLINK-29775 branch 2 times, most recently from b49f410 to a49f5f9 Compare August 7, 2026 18:42
@spuru9
spuru9 marked this pull request as ready for review August 7, 2026 18:49
…ksdb

Migrates the remaining JUnit 4 tests in flink-statebackend-rocksdb to
JUnit 5 and converts their assertions to AssertJ, following the sibling
flink-statebackend-forst migration (FLINK-39767) where the tests overlap.

- @Rule/@ClassRule TemporaryFolder -> @tempdir (method parameter where a
  test needs a single directory, TempDirUtils otherwise)
- @RunWith(Parameterized) -> @ParameterizedTest + @valuesource
- @test(expected=...) and try/fail/catch -> assertThatThrownBy
- org.junit.Assert and Hamcrest matchers -> AssertJ
- org.junit.Assume -> AssertJ assumptions
- dropped `extends TestLogger` and redundant `public` modifiers

RocksDBExtension and RocksDBKeyedStateBackendTestFactory are test
utilities rather than tests; they kept their JUnit 4 TemporaryFolder and
are switched to plain temp directories / java.nio.file.Path so the module
no longer depends on JUnit 4 at all.

Generated-by: Claude Code (claude-opus-5)
@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Aug 8, 2026
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.

2 participants