From fbb492876048e9260b8b93056646de34133c9ad9 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Sun, 3 Nov 2019 21:34:32 +0300 Subject: [PATCH 1/2] Improve stability --- .../catalyst/util/DateTimeUtilsSuite.scala | 60 ++++++++++++------- .../sql/util/TimestampFormatterSuite.scala | 38 +++++++----- 2 files changed, 60 insertions(+), 38 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala index f9deb4d05eab..348b752be85b 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala @@ -19,7 +19,7 @@ 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 @@ -606,33 +606,47 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers { test("special timestamp values") { DateTimeTestUtils.outstandingZoneIds.foreach { 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) - 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) - val today = instantToMicros(localToday.toInstant) - toTimestamp("Today ", zoneId).get should be (today +- tolerance) - val tomorrow = instantToMicros(localToday.plusDays(1).toInstant) - toTimestamp(" tomorrow CET ", zoneId).get should be (tomorrow +- tolerance) + withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { + // 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. + retry(1) { + val tolerance = TimeUnit.SECONDS.toMicros(30) + + assert(toTimestamp("Epoch", zoneId).get === 0) + 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) + val today = instantToMicros(localToday.toInstant) + toTimestamp("Today ", zoneId).get should be(today +- tolerance) + val tomorrow = instantToMicros(localToday.plusDays(1).toInstant) + toTimestamp(" tomorrow CET ", zoneId).get should be(tomorrow +- tolerance) + } + } } } test("special date values") { DateTimeTestUtils.outstandingZoneIds.foreach { zoneId => - assert(toDate("epoch", zoneId).get === 0) - val today = localDateToDays(LocalDate.now(zoneId)) - assert(toDate("YESTERDAY", zoneId).get === today - 1) - assert(toDate(" Now ", zoneId).get === today) - assert(toDate("now UTC", zoneId) === None) // "now" does not accept time zones - assert(toDate("today", zoneId).get === today) - assert(toDate("tomorrow CET ", zoneId).get === today + 1) + withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { + // 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. + retry(1) { + assert(toDate("epoch", zoneId).get === 0) + val today = localDateToDays(LocalDate.now(zoneId)) + assert(toDate("YESTERDAY", zoneId).get === today - 1) + assert(toDate(" Now ", zoneId).get === today) + assert(toDate("now UTC", zoneId) === None) // "now" does not accept time zones + assert(toDate("today", zoneId).get === today) + assert(toDate("tomorrow CET ", zoneId).get === today + 1) + } + } } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala index 84581c0badd8..93f00bc78e57 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala @@ -17,7 +17,7 @@ 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 @@ -140,21 +140,29 @@ class TimestampFormatterSuite extends SparkFunSuite with SQLHelper with Matchers 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) - 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) + withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { + // 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. + retry(1) { + val formatter = TimestampFormatter(zoneId) + val tolerance = TimeUnit.SECONDS.toMicros(30) + + 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) + } + } } } } From 44aeda8aeb298f721a5732c77f96ce4530508d74 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Sun, 3 Nov 2019 22:04:26 +0300 Subject: [PATCH 2/2] Extract common code to SQLHelper --- .../spark/sql/catalyst/plans/SQLHelper.scala | 22 +++++++ .../catalyst/util/DateTimeUtilsSuite.scala | 65 ++++++++----------- .../spark/sql/util/DateFormatterSuite.scala | 19 +++--- .../sql/util/TimestampFormatterSuite.scala | 44 +++++-------- 4 files changed, 72 insertions(+), 78 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/SQLHelper.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/SQLHelper.scala index 4d869d79ad59..d213743946e7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/SQLHelper.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/SQLHelper.scala @@ -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 @@ -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) + } + } + } + } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala index 348b752be85b..c1618e78559d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala @@ -26,11 +26,12 @@ 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() @@ -605,48 +606,34 @@ class DateTimeUtilsSuite extends SparkFunSuite with Matchers { } test("special timestamp values") { - DateTimeTestUtils.outstandingZoneIds.foreach { zoneId => - withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { - // 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. - retry(1) { - val tolerance = TimeUnit.SECONDS.toMicros(30) - - assert(toTimestamp("Epoch", zoneId).get === 0) - 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) - val today = instantToMicros(localToday.toInstant) - toTimestamp("Today ", zoneId).get should be(today +- tolerance) - val tomorrow = instantToMicros(localToday.plusDays(1).toInstant) - toTimestamp(" tomorrow CET ", zoneId).get should be(tomorrow +- tolerance) - } - } + testSpecialDatetimeValues { zoneId => + val tolerance = TimeUnit.SECONDS.toMicros(30) + + assert(toTimestamp("Epoch", zoneId).get === 0) + 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) + val today = instantToMicros(localToday.toInstant) + toTimestamp("Today ", zoneId).get should be(today +- tolerance) + val tomorrow = instantToMicros(localToday.plusDays(1).toInstant) + toTimestamp(" tomorrow CET ", zoneId).get should be(tomorrow +- tolerance) } } test("special date values") { - DateTimeTestUtils.outstandingZoneIds.foreach { zoneId => - withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { - // 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. - retry(1) { - assert(toDate("epoch", zoneId).get === 0) - val today = localDateToDays(LocalDate.now(zoneId)) - assert(toDate("YESTERDAY", zoneId).get === today - 1) - assert(toDate(" Now ", zoneId).get === today) - assert(toDate("now UTC", zoneId) === None) // "now" does not accept time zones - assert(toDate("today", zoneId).get === today) - assert(toDate("tomorrow CET ", zoneId).get === today + 1) - } - } + testSpecialDatetimeValues { zoneId => + assert(toDate("epoch", zoneId).get === 0) + val today = localDateToDays(LocalDate.now(zoneId)) + assert(toDate("YESTERDAY", zoneId).get === today - 1) + assert(toDate(" Now ", zoneId).get === today) + assert(toDate("now UTC", zoneId) === None) // "now" does not accept time zones + assert(toDate("today", zoneId).get === today) + assert(toDate("tomorrow CET ", zoneId).get === today + 1) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/DateFormatterSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/DateFormatterSuite.scala index 291d40a9e84d..d617b1c1d823 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/DateFormatterSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/DateFormatterSuite.scala @@ -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) } } } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala index 93f00bc78e57..6107a15f5c42 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/util/TimestampFormatterSuite.scala @@ -25,8 +25,7 @@ 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 { @@ -137,33 +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) + testSpecialDatetimeValues { zoneId => + val formatter = TimestampFormatter(zoneId) + val tolerance = TimeUnit.SECONDS.toMicros(30) - withClue(s"zoneId = $zoneId, current time = ${LocalDateTime.now(zoneId)}") { - // 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. - retry(1) { - val formatter = TimestampFormatter(zoneId) - val tolerance = TimeUnit.SECONDS.toMicros(30) - - 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) - } - } - } + 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) } } }