[SPARK-40467][SS] Split FlatMapGroupsWithState down to multiple test suites #37907
Conversation
This file contains 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
HeartSaVioR
commented
Sep 16, 2022
| @@ -78,416 +78,6 @@ class FlatMapGroupsWithStateSuite extends StateStoreMetricsTest { | |||
| } | |||
| } | |||
|
|
|||
| test("SPARK-35800: ensure TestGroupState creates instances the same as prod") { | |||
There was a problem hiding this comment.
Lines in L81 to L490 are moved out to GroupStateSuite.
| @@ -1268,258 +858,6 @@ class FlatMapGroupsWithStateSuite extends StateStoreMetricsTest { | |||
| assert(e.getMessage === "The output mode of function should be append or update") | |||
| } | |||
|
|
|||
| import testImplicits._ | |||
There was a problem hiding this comment.
Lines in L1271 to L1635 (except a single testcase which isn't related) are moved out to FlatMapGroupsWithStateWithInitialStateSuite. L1790 to L1809 are also moved out as they are only referred by L1271 to L1635.
| ) | ||
| } | ||
|
|
||
| def testWithAllStateVersions(name: String)(func: => Unit): Unit = { |
There was a problem hiding this comment.
This is only one duplication we will have after refactoring. I just copied over the code rather than dealing with inheritance / util class as it's just about less than 10 lines.
HyukjinKwon
approved these changes
Sep 16, 2022
|
Thanks! Merging to master. (The last commit only removed unused imports.) |
LuciferYang
pushed a commit
to LuciferYang/spark
that referenced
this pull request
Sep 20, 2022
…suites ### What changes were proposed in this pull request? This PR proposes to split the FlatMapGroupsWithStateSuite into three pieces: 1. GroupStateSuite <- test the functionality with (Test)GroupState implementation 2. FlatMapGroupsWithStateWithInitialStateSuite <- test E2E cases which are specific to initial state 3. FlatMapGroupsWithStateSuite <- all other cases (E2E cases which don't leverage initial state) The change is pure extraction - it's cut and paste and no additional code change has been introduced. ### Why are the changes needed? This would help to maintain the test suite FlatMapGroupsWithStateSuite as it's quite huge. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Refactored test suites. Closes apache#37907 from HeartSaVioR/SPARK-40467. Authored-by: Jungtaek Lim <kabhwan.opensource@gmail.com> Signed-off-by: Jungtaek Lim <kabhwan.opensource@gmail.com>
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 changes were proposed in this pull request?
This PR proposes to split the FlatMapGroupsWithStateSuite into three pieces:
The change is pure extraction - it's cut and paste and no additional code change has been introduced.
Why are the changes needed?
This would help to maintain the test suite FlatMapGroupsWithStateSuite as it's quite huge.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Refactored test suites.