Skip to content

Commit 186477c

Browse files
xkrogenMridul Muralidharan
authored andcommitted
[SPARK-35263][TEST] Refactor ShuffleBlockFetcherIteratorSuite to reduce duplicated code
### What changes were proposed in this pull request? Introduce new shared methods to `ShuffleBlockFetcherIteratorSuite` to replace copy-pasted code. Use modern, Scala-like Mockito `Answer` syntax. ### Why are the changes needed? `ShuffleFetcherBlockIteratorSuite` has tons of duplicate code, like https://github.com/apache/spark/blob/0494dc90af48ce7da0625485a4dc6917a244d580/core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala#L172-L185 . It's challenging to tell what the interesting parts are vs. what is just being set to some default/unused value. Similarly but not as bad, there are many calls like the following ``` verify(transfer, times(1)).fetchBlocks(any(), any(), any(), any(), any(), any()) when(transfer.fetchBlocks(any(), any(), any(), any(), any(), any())).thenAnswer ... ``` These changes result in about 10% reduction in both lines and characters in the file: ```bash # Before > wc core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala 1063 3950 43201 core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala # After > wc core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala 928 3609 39053 core/src/test/scala/org/apache/spark/storage/ShuffleBlockFetcherIteratorSuite.scala ``` It also helps readability, e.g.: ``` val iterator = createShuffleBlockIteratorWithDefaults( transfer, blocksByAddress, maxBytesInFlight = 1000L ) ``` Now I can clearly tell that `maxBytesInFlight` is the main parameter we're interested in here. ### Does this PR introduce _any_ user-facing change? No, test only. There aren't even any behavior changes, just refactoring. ### How was this patch tested? Unit tests pass. Closes apache#32389 from xkrogen/xkrogen-spark-35263-refactor-shuffleblockfetcheriteratorsuite. Authored-by: Erik Krogen <[email protected]> Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
1 parent 8c70c17 commit 186477c

File tree

1 file changed

+245
-444
lines changed

1 file changed

+245
-444
lines changed

0 commit comments

Comments
 (0)