Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22631#discussion_r223228194
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala
---
@@ -110,7 +112,7 @@ class CastSuite extends SparkFunSuite with
ExpressionEvalHelper {
}
test("cast string to timestamp") {
- for (tz <- ALL_TIMEZONES) {
+ for (tz <- Random.shuffle(ALL_TIMEZONES).take(50)) {
--- End diff --
Surely not by design? tests need to be deterministic, or else what's the
value? failures can't be reproduced. (I know that in practice many things are
hard to make deterministic.)
Of course, if you're worried that we might not be testing an important
case, we have to test it. We can't just not test it sometimes to make some
tests run a little faster.
Testing just 3 timezones might be fine too; I don't know. Testing 50
randomly seems suboptimal in all cases.
I'll open a PR to try simply testing in parallel instead.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
