You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2020/03/02 23:23:13 UTC
[spark] branch branch-3.0 updated: [SPARK-31003][TESTS] Fix
incorrect uses of assume() in tests
This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new 8cb23f0 [SPARK-31003][TESTS] Fix incorrect uses of assume() in tests
8cb23f0 is described below
commit 8cb23f0cb5b20b7e49fdd16c52d6451e901d9a7a
Author: Josh Rosen <ro...@gmail.com>
AuthorDate: Mon Mar 2 15:20:45 2020 -0800
[SPARK-31003][TESTS] Fix incorrect uses of assume() in tests
### What changes were proposed in this pull request?
This patch fixes several incorrect uses of `assume()` in our tests.
If a call to `assume(condition)` fails then it will cause the test to be marked as skipped instead of failed: this feature allows test cases to be skipped if certain prerequisites are missing. For example, we use this to skip certain tests when running on Windows (or when Python dependencies are unavailable).
In contrast, `assert(condition)` will fail the test if the condition doesn't hold.
If `assume()` is accidentally substituted for `assert()`then the resulting test will be marked as skipped in cases where it should have failed, undermining the purpose of the test.
This patch fixes several such cases, replacing certain `assume()` calls with `assert()`.
Credit to ahirreddy for spotting this problem.
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
Existing tests.
Closes #27754 from JoshRosen/fix-assume-vs-assert.
Lead-authored-by: Josh Rosen <ro...@gmail.com>
Co-authored-by: Josh Rosen <jo...@databricks.com>
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
(cherry picked from commit f0010c81e2ef9b8859b39917bb62b48d739a4a22)
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
.../org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala | 2 +-
sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala | 2 +-
.../scala/org/apache/spark/sql/execution/command/DDLSuite.scala | 4 ++--
.../src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala | 6 +++---
.../test/scala/org/apache/spark/sql/sources/BucketedReadSuite.scala | 2 +-
.../scala/org/apache/spark/sql/sources/BucketedWriteSuite.scala | 2 +-
.../scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala | 6 +++---
.../src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala | 2 +-
.../apache/spark/sql/sources/BucketedReadWithHiveSupportSuite.scala | 2 +-
.../spark/sql/sources/BucketedWriteWithHiveSupportSuite.scala | 2 +-
10 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
index 94e251d..4488902 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/OrderingSuite.scala
@@ -106,7 +106,7 @@ class OrderingSuite extends SparkFunSuite with ExpressionEvalHelper {
StructField("a", dataType, nullable = true) ::
StructField("b", dataType, nullable = true) :: Nil)
val maybeDataGenerator = RandomDataGenerator.forType(rowType, nullable = false)
- assume(maybeDataGenerator.isDefined)
+ assert(maybeDataGenerator.isDefined)
val randGenerator = maybeDataGenerator.get
val toCatalyst = CatalystTypeConverters.createToCatalystConverter(rowType)
for (_ <- 1 to 50) {
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala
index cd2c681..8189353 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/CachedTableSuite.scala
@@ -195,7 +195,7 @@ class CachedTableSuite extends QueryTest with SQLTestUtils
}
test("SPARK-1669: cacheTable should be idempotent") {
- assume(!spark.table("testData").logicalPlan.isInstanceOf[InMemoryRelation])
+ assert(!spark.table("testData").logicalPlan.isInstanceOf[InMemoryRelation])
spark.catalog.cacheTable("testData")
assertCached(spark.table("testData"))
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
index 6c824c2..5a67dce 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala
@@ -1033,7 +1033,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
df.write.insertInto("students")
spark.catalog.cacheTable("students")
checkAnswer(spark.table("students"), df)
- assume(spark.catalog.isCached("students"), "bad test: table was not cached in the first place")
+ assert(spark.catalog.isCached("students"), "bad test: table was not cached in the first place")
sql("ALTER TABLE students RENAME TO teachers")
sql("CREATE TABLE students (age INT, name STRING) USING parquet")
// Now we have both students and teachers.
@@ -1959,7 +1959,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
Seq("json", "parquet").foreach { format =>
withTable("rectangles") {
data.write.format(format).saveAsTable("rectangles")
- assume(spark.table("rectangles").collect().nonEmpty,
+ assert(spark.table("rectangles").collect().nonEmpty,
"bad test; table was empty to begin with")
sql("TRUNCATE TABLE rectangles")
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
index d6a1fde..4630a42 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/CatalogSuite.scala
@@ -89,9 +89,9 @@ class CatalogSuite extends SharedSparkSession {
val columns = dbName
.map { db => spark.catalog.listColumns(db, tableName) }
.getOrElse { spark.catalog.listColumns(tableName) }
- assume(tableMetadata.schema.nonEmpty, "bad test")
- assume(tableMetadata.partitionColumnNames.nonEmpty, "bad test")
- assume(tableMetadata.bucketSpec.isDefined, "bad test")
+ assert(tableMetadata.schema.nonEmpty, "bad test")
+ assert(tableMetadata.partitionColumnNames.nonEmpty, "bad test")
+ assert(tableMetadata.bucketSpec.isDefined, "bad test")
assert(columns.collect().map(_.name).toSet == tableMetadata.schema.map(_.name).toSet)
val bucketColumnNames = tableMetadata.bucketSpec.map(_.bucketColumnNames).getOrElse(Nil).toSet
columns.collect().foreach { col =>
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedReadSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedReadSuite.scala
index c7266c8..b54f4f2 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedReadSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedReadSuite.scala
@@ -42,7 +42,7 @@ import org.apache.spark.util.collection.BitSet
class BucketedReadWithoutHiveSupportSuite extends BucketedReadSuite with SharedSparkSession {
protected override def beforeAll(): Unit = {
super.beforeAll()
- assume(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "in-memory")
+ assert(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "in-memory")
}
}
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedWriteSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedWriteSuite.scala
index 9713de9..a410f32 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedWriteSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/sources/BucketedWriteSuite.scala
@@ -33,7 +33,7 @@ import org.apache.spark.sql.test.{SharedSparkSession, SQLTestUtils}
class BucketedWriteWithoutHiveSupportSuite extends BucketedWriteSuite with SharedSparkSession {
protected override def beforeAll(): Unit = {
super.beforeAll()
- assume(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "in-memory")
+ assert(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "in-memory")
}
override protected def fileFormatsToTest: Seq[String] = Seq("parquet", "json")
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
index 7adc68d..08d2506 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
@@ -981,8 +981,8 @@ class HiveDDLSuite
val expectedSerdePropsString =
expectedSerdeProps.map { case (k, v) => s"'$k'='$v'" }.mkString(", ")
val oldPart = catalog.getPartition(TableIdentifier("boxes"), Map("width" -> "4"))
- assume(oldPart.storage.serde != Some(expectedSerde), "bad test: serde was already set")
- assume(oldPart.storage.properties.filterKeys(expectedSerdeProps.contains) !=
+ assert(oldPart.storage.serde != Some(expectedSerde), "bad test: serde was already set")
+ assert(oldPart.storage.properties.filterKeys(expectedSerdeProps.contains) !=
expectedSerdeProps, "bad test: serde properties were already set")
sql(s"""ALTER TABLE boxes PARTITION (width=4)
| SET SERDE '$expectedSerde'
@@ -1735,7 +1735,7 @@ class HiveDDLSuite
Seq("json", "parquet").foreach { format =>
withTable("rectangles") {
data.write.format(format).saveAsTable("rectangles")
- assume(spark.table("rectangles").collect().nonEmpty,
+ assert(spark.table("rectangles").collect().nonEmpty,
"bad test; table was empty to begin with")
sql("TRUNCATE TABLE rectangles")
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
index 222244a..868bc27 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/test/TestHive.scala
@@ -212,7 +212,7 @@ private[hive] class TestHiveSparkSession(
}
}
- assume(sc.conf.get(CATALOG_IMPLEMENTATION) == "hive")
+ assert(sc.conf.get(CATALOG_IMPLEMENTATION) == "hive")
@transient
override lazy val sharedState: TestHiveSharedState = {
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedReadWithHiveSupportSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedReadWithHiveSupportSuite.scala
index f277f99..35dab79 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedReadWithHiveSupportSuite.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedReadWithHiveSupportSuite.scala
@@ -23,6 +23,6 @@ import org.apache.spark.sql.internal.StaticSQLConf.CATALOG_IMPLEMENTATION
class BucketedReadWithHiveSupportSuite extends BucketedReadSuite with TestHiveSingleton {
protected override def beforeAll(): Unit = {
super.beforeAll()
- assume(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "hive")
+ assert(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "hive")
}
}
diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedWriteWithHiveSupportSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedWriteWithHiveSupportSuite.scala
index 454e2f6..bdbdcc2 100644
--- a/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedWriteWithHiveSupportSuite.scala
+++ b/sql/hive/src/test/scala/org/apache/spark/sql/sources/BucketedWriteWithHiveSupportSuite.scala
@@ -23,7 +23,7 @@ import org.apache.spark.sql.internal.StaticSQLConf.CATALOG_IMPLEMENTATION
class BucketedWriteWithHiveSupportSuite extends BucketedWriteSuite with TestHiveSingleton {
protected override def beforeAll(): Unit = {
super.beforeAll()
- assume(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "hive")
+ assert(spark.sparkContext.conf.get(CATALOG_IMPLEMENTATION) == "hive")
}
override protected def fileFormatsToTest: Seq[String] = Seq("parquet", "orc")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org