Skip to content

[SPARK-29736][TESTS] Improve stability of tests for special datetime values #26380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MaxGekk wants to merge 2 commits into from
Closed

[SPARK-29736][TESTS] Improve stability of tests for special datetime values #26380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbb4928
Improve stability
MaxGekk Nov 3, 2019
44aeda8
Extract common code to SQLHelper
MaxGekk Nov 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/SQLHelper.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@
package org.apache.spark.sql.catalyst.plans

import java.io.File
import java.time.ZoneId

import scala.util.control.NonFatal

import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.util.DateTimeTestUtils
import org.apache.spark.sql.catalyst.util.DateTimeUtils.getZoneId
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.util.Utils

Expand Down Expand Up @@ -61,4 +66,21 @@ trait SQLHelper {
path.delete()
try f(path) finally Utils.deleteRecursively(path)
}


def testSpecialDatetimeValues[T](test: ZoneId => T): Unit = {
DateTimeTestUtils.outstandingTimezonesIds.foreach { timeZone =>
withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> timeZone) {
val zoneId = getZoneId(timeZone)
// The test can fail around midnight if it gets the reference value
// before midnight but tested code resolves special value after midnight.
// Retry can guarantee that both values were taken on the same day.
try {
test(zoneId)
} catch {
case NonFatal(_) => test(zoneId)
Copy link
Contributor

@cloud-fan cloud-fan Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we set a threshold like 3? otherwise we may hit dead loop for real failures.

Copy link
Member Author

@MaxGekk MaxGekk Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I don't understand how such loop could happen but this is not recursive code. test can be invoke maximum 2 times.

Copy link
Contributor

@cloud-fan cloud-fan Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I mis-looked. It's not recursive. LGTM

}
}
}
}
}
19 changes: 10 additions & 9 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,19 @@ package org.apache.spark.sql.catalyst.util

import java.sql.{Date, Timestamp}
import java.text.SimpleDateFormat
import java.time.{LocalDate, LocalDateTime, LocalTime, ZoneId, ZoneOffset}
import java.time.{Instant, LocalDate, LocalDateTime, LocalTime, ZoneId, ZoneOffset}
import java.util.{Locale, TimeZone}
import java.util.concurrent.TimeUnit

import org.scalatest.Matchers

import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.plans.SQLHelper
import org.apache.spark.sql.catalyst.util.DateTimeTestUtils._
import org.apache.spark.sql.catalyst.util.DateTimeUtils._
import org.apache.spark.unsafe.types.{CalendarInterval, UTF8String}

class DateTimeUtilsSuite extends SparkFunSuite with Matchers {
class DateTimeUtilsSuite extends SparkFunSuite with Matchers with SQLHelper {

val TimeZonePST = TimeZone.getTimeZone("PST")
private def defaultZoneId = ZoneId.systemDefault()
Expand Down Expand Up @@ -605,27 +606,27 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers {
}

test("special timestamp values") {
DateTimeTestUtils.outstandingZoneIds.foreach { zoneId =>
testSpecialDatetimeValues { zoneId =>
val tolerance = TimeUnit.SECONDS.toMicros(30)

assert(toTimestamp("Epoch", zoneId).get === 0)
val now = instantToMicros(LocalDateTime.now(zoneId).atZone(zoneId).toInstant)
toTimestamp("NOW", zoneId).get should be (now +- tolerance)
val now = instantToMicros(Instant.now())
toTimestamp("NOW", zoneId).get should be(now +- tolerance)
assert(toTimestamp("now UTC", zoneId) === None)
val localToday = LocalDateTime.now(zoneId)
.`with`(LocalTime.MIDNIGHT)
.atZone(zoneId)
val yesterday = instantToMicros(localToday.minusDays(1).toInstant)
toTimestamp(" Yesterday", zoneId).get should be (yesterday +- tolerance)
toTimestamp(" Yesterday", zoneId).get should be(yesterday +- tolerance)
val today = instantToMicros(localToday.toInstant)
toTimestamp("Today ", zoneId).get should be (today +- tolerance)
toTimestamp("Today ", zoneId).get should be(today +- tolerance)
val tomorrow = instantToMicros(localToday.plusDays(1).toInstant)
toTimestamp(" tomorrow CET ", zoneId).get should be (tomorrow +- tolerance)
toTimestamp(" tomorrow CET ", zoneId).get should be(tomorrow +- tolerance)
}
}

test("special date values") {
DateTimeTestUtils.outstandingZoneIds.foreach { zoneId =>
testSpecialDatetimeValues { zoneId =>
assert(toDate("epoch", zoneId).get === 0)
val today = localDateToDays(LocalDate.now(zoneId))
assert(toDate("YESTERDAY", zoneId).get === today - 1)
Expand Down
19 changes: 8 additions & 11 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/util/DateFormatterSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,15 @@ class DateFormatterSuite extends SparkFunSuite with SQLHelper {
}

test("special date values") {
DateTimeTestUtils.outstandingTimezonesIds.foreach { timeZone =>
withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> timeZone) {
val zoneId = getZoneId(timeZone)
val formatter = DateFormatter(zoneId)
testSpecialDatetimeValues { zoneId =>
val formatter = DateFormatter(zoneId)

assert(formatter.parse("EPOCH") === 0)
val today = localDateToDays(LocalDate.now(zoneId))
assert(formatter.parse("Yesterday") === today - 1)
assert(formatter.parse("now") === today)
assert(formatter.parse("today ") === today)
assert(formatter.parse("tomorrow UTC") === today + 1)
}
assert(formatter.parse("EPOCH") === 0)
val today = localDateToDays(LocalDate.now(zoneId))
assert(formatter.parse("Yesterday") === today - 1)
assert(formatter.parse("now") === today)
assert(formatter.parse("today ") === today)
assert(formatter.parse("tomorrow UTC") === today + 1)
}
}
}
38 changes: 17 additions & 21 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@

package org.apache.spark.sql.util

import java.time.{LocalDateTime, LocalTime, ZoneOffset}
import java.time.{Instant, LocalDateTime, LocalTime, ZoneOffset}
import java.util.concurrent.TimeUnit

import org.scalatest.Matchers

import org.apache.spark.SparkFunSuite
import org.apache.spark.sql.catalyst.plans.SQLHelper
import org.apache.spark.sql.catalyst.util.{DateTimeTestUtils, DateTimeUtils, TimestampFormatter}
import org.apache.spark.sql.catalyst.util.DateTimeUtils.{getZoneId, instantToMicros}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.catalyst.util.DateTimeUtils.instantToMicros

class TimestampFormatterSuite extends SparkFunSuite with SQLHelper with Matchers {

Expand Down Expand Up @@ -137,25 +136,22 @@ class TimestampFormatterSuite extends SparkFunSuite with SQLHelper with Matchers
}

test("special timestamp values") {
DateTimeTestUtils.outstandingTimezonesIds.foreach { timeZone =>
withSQLConf(SQLConf.SESSION_LOCAL_TIMEZONE.key -> timeZone) {
val zoneId = getZoneId(timeZone)
val formatter = TimestampFormatter(zoneId)
val tolerance = TimeUnit.SECONDS.toMicros(30)
testSpecialDatetimeValues { zoneId =>
val formatter = TimestampFormatter(zoneId)
val tolerance = TimeUnit.SECONDS.toMicros(30)

assert(formatter.parse("EPOCH") === 0)
val now = instantToMicros(LocalDateTime.now(zoneId).atZone(zoneId).toInstant)
formatter.parse("now") should be (now +- tolerance)
val localToday = LocalDateTime.now(zoneId)
.`with`(LocalTime.MIDNIGHT)
.atZone(zoneId)
val yesterday = instantToMicros(localToday.minusDays(1).toInstant)
formatter.parse("yesterday CET") should be (yesterday +- tolerance)
val today = instantToMicros(localToday.toInstant)
formatter.parse(" TODAY ") should be (today +- tolerance)
val tomorrow = instantToMicros(localToday.plusDays(1).toInstant)
formatter.parse("Tomorrow ") should be (tomorrow +- tolerance)
}
assert(formatter.parse("EPOCH") === 0)
val now = instantToMicros(Instant.now())
formatter.parse("now") should be(now +- tolerance)
val localToday = LocalDateTime.now(zoneId)
.`with`(LocalTime.MIDNIGHT)
.atZone(zoneId)
val yesterday = instantToMicros(localToday.minusDays(1).toInstant)
formatter.parse("yesterday CET") should be(yesterday +- tolerance)
val today = instantToMicros(localToday.toInstant)
formatter.parse(" TODAY ") should be(today +- tolerance)
val tomorrow = instantToMicros(localToday.plusDays(1).toInstant)
formatter.parse("Tomorrow ") should be(tomorrow +- tolerance)
}
}
}