[FLINK-40207][table-runtime] Modernize BinaryStringDataTest with JUnit 5 - #29023
Open
raminqaf wants to merge 8 commits into
Open
[FLINK-40207][table-runtime] Modernize BinaryStringDataTest with JUnit 5#29023raminqaf wants to merge 8 commits into
raminqaf wants to merge 8 commits into
Conversation
snuyanzin
reviewed
Aug 26, 2026
snuyanzin
reviewed
Aug 26, 2026
Rework BinaryStringDataTest to plain JUnit 5 instead of the custom ParameterizedTestExtension. The memory-layout axis is now a method-level parameter: layout-dependent tests are @ParameterizedTest over @EnumSource(Mode.class) and build their strings through a static fromString(Mode, String), while layout-independent tests become plain @test instead of running four identical times. Tests are grouped into @nested classes by concern (Basics, Comparison, Search, Manipulation, Conversion, Encoding). The string-to-decimal cases move to a shared list fed to a @MethodSource, so each case is now its own reported invocation. The two oversized methods are split into focused tests: the numeric method into integral, approximate-special-value and binary-row tests, and the decimal method into string and binary-row tests. No assertions are dropped. Every prior scenario maps forward and the full class passes.
snuyanzin
reviewed
Aug 26, 2026
raminqaf
force-pushed
the
FLINK-40207-junit5-tests
branch
from
August 26, 2026 11:26
73bdd40 to
a066399
Compare
Collaborator
snuyanzin
reviewed
Aug 26, 2026
snuyanzin
reviewed
Aug 26, 2026
snuyanzin
reviewed
Aug 26, 2026
snuyanzin
reviewed
Aug 28, 2026
snuyanzin
reviewed
Aug 28, 2026
Split toApproximateSpecialValues into separate parameterized tests for positive infinity, negative infinity, NaN and invalid input, each driven by a @MethodSource that crosses the modes with the input strings. Turn skipWrongFirstByte into a @valuesource test and testDecodeWithIllegalUtf8Bytes into a @MethodSource test over the decode entry points, so each case is its own invocation instead of a loop.
snuyanzin
reviewed
Aug 28, 2026
Turn the remaining loop-based binary-row decimal test into a @MethodSource parameterized test. Each case writes a single-column BinaryRowData, reads it back and asserts, so every decimal string is its own invocation instead of two loops over a shared row.
raminqaf
force-pushed
the
FLINK-40207-junit5-tests
branch
from
August 28, 2026 10:15
cfc9920 to
a51709f
Compare
snuyanzin
reviewed
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
Test-only follow-up to the FLINK-40207 cast fix.
BinaryStringDataTestused Flink's customParameterizedTestExtension(constructor-injectedMode,@TestTemplate,@Parameters), and several methods bundled many unrelated scenarios into one body. This reworks it to plain JUnit 5 for readability and more granular reporting. There is no production code change and no behavior change under test.Brief change log
ParameterizedTestExtensionwith plain JUnit 5. The memory-layout axis is now a method parameter: layout-dependent tests are@ParameterizedTestover@EnumSource(Mode.class)and build strings through a staticfromString(Mode, String).@Test(run once) instead of running four identical times. This includes the two binary-row blocks previously nested inside the numeric and decimal tests, which never used the layout helper.@Nestedclasses by concern:Basics,Comparison,Search,Manipulation,Conversion,Encoding.@MethodSource, so each case is its own reported invocation.Verifying this change
This change is test-only and is verified by the tests themselves. The full class passes (279 invocations across the six nested groups, 0 failures). No assertions were dropped: every prior scenario maps forward, and the 49 decimal cases are unchanged.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
Was generative AI tooling used to co-author this PR?
Generated-by: Opus 4.8