[SPARK-54115][TESTS][FOLLOW-UP] Refine org.apache.spark.util.UtilsSuite
#53192
Conversation
|
The AQE test failure is unrelated, thanks, merging to master/4.1! |
…ite` ### What changes were proposed in this pull request? ### Why are the changes needed? > Use doReturn() in those rare occasions when you cannot use when(Object) `when` is preferred in the official doc https://javadoc.io/static/org.mockito/mockito-core/5.12.0/org/mockito/Mockito.html#when(T) ### Does this PR introduce _any_ user-facing change? no, test-only ### How was this patch tested? ci ### Was this patch authored or co-authored using generative AI tooling? no Closes #53192 from zhengruifeng/spark_54115_test_followup. Authored-by: Ruifeng Zheng <ruifengz@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d14209c) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
|
|
||
| val sorted = Seq(connectExecuteOp1T, connectExecuteOp2T, task1T, task2T) | ||
| .sorted(Utils.invokePrivate(threadInfoOrderingMethod())) | ||
| .sorted(Utils.threadInfoOrdering) |
There was a problem hiding this comment.
Hmm... this looks simple, but I was told to prefer using invokePrivate to avoid exposing private methods/fields, not sure which is the best practice for this case.
cc @LuciferYang for more inputs
There was a problem hiding this comment.
private[spark] is not public.
There was a problem hiding this comment.
This appears to be a new piece of code from the 4.1 cycle rather than legacy code. If we also consider it acceptable for third-party libraries to use threadInfoOrdering within the spark package, then this modification is acceptable.
There was a problem hiding this comment.
third-party lib can do whatever they want using reflection, the point is what Spark guarantees/promises. Apparently, there is no compatibility guarantee/promise for private[spark] APIs.
There was a problem hiding this comment.
The reasoning is correct.
…ite` ### What changes were proposed in this pull request? ### Why are the changes needed? > Use doReturn() in those rare occasions when you cannot use when(Object) `when` is preferred in the official doc https://javadoc.io/static/org.mockito/mockito-core/5.12.0/org/mockito/Mockito.html#when(T) ### Does this PR introduce _any_ user-facing change? no, test-only ### How was this patch tested? ci ### Was this patch authored or co-authored using generative AI tooling? no Closes apache#53192 from zhengruifeng/spark_54115_test_followup. Authored-by: Ruifeng Zheng <ruifengz@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
4 participants
What changes were proposed in this pull request?
Why are the changes needed?
whenis preferred in the official doc https://javadoc.io/static/org.mockito/mockito-core/5.12.0/org/mockito/Mockito.html#when(T)Does this PR introduce any user-facing change?
no, test-only
How was this patch tested?
ci
Was this patch authored or co-authored using generative AI tooling?
no