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 <[email protected]>
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 <[email protected]>
Co-authored-by: Josh Rosen <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit f0010c81e2ef9b8859b39917bb62b48d739a4a22)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../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: [email protected]
For additional commands, e-mail: [email protected]
