[SPARK-33789][SQL][TESTS] Refactor unified V1 and V2 datasource tests #30779

MaxGekk · 2020-12-15T11:48:55Z

What changes were proposed in this pull request?

Move common utility functions such as test(), withNsTable() and checkPartitions() to DDLCommandTestUtils.
Place common settings such as version, catalog, defaultUsing, sparkConf to CommandSuiteBase.

Why are the changes needed?

To improve code maintenance of the unified tests.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running the affected test suites:

$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowPartitionsSuite"
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowTablesSuite"
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableAddPartitionSuite"
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *AlterTableDropPartitionSuite"

...src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala

…/AlterTableDropPartitionSuiteBase.scala Co-authored-by: John Bampton <jbampton@users.noreply.github.com>

SparkQA · 2020-12-15T12:45:22Z

Test build #132826 has finished for PR 30779 at commit 89e3898.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-12-15T13:49:28Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37430/

cloud-fan · 2020-12-15T13:51:51Z

.../src/test/scala/org/apache/spark/sql/execution/command/AlterTableAddPartitionSuiteBase.scala

-  protected def catalog: String
+trait AlterTableAddPartitionSuiteBase extends QueryTest with DDLCommandTestUtils {
+  override val command = "ALTER TABLE .. ADD PARTITION"
  protected def defaultUsing: String


why we still have it here?

cloud-fan · 2020-12-15T13:51:59Z

...src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala

-  protected def version: String
+trait AlterTableDropPartitionSuiteBase extends QueryTest with DDLCommandTestUtils {
+  override val command = "ALTER TABLE .. DROP PARTITION"
  protected def catalog: String


cloud-fan · 2020-12-15T13:53:06Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandTestUtils.scala

+    super.test(s"$command $version: " + testName, testTags: _*)(testFun)
+  }
+
+  protected def withNsTable(ns: String, tableName: String, cat: String = catalog)


since we are here, shall we make the name clearer? like withNamespaceAndTable

Renamed to withNamespaceAndTable

cloud-fan · 2020-12-15T13:53:28Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowPartitionsSuiteBase.scala

-  protected def version: String
+trait ShowPartitionsSuiteBase extends QueryTest with DDLCommandTestUtils {
+  override val command = "SHOW PARTITIONS"
  protected def catalog: String


cloud-fan · 2020-12-15T14:10:57Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLCommandTestUtils.scala

+    super.test(s"$command $version: " + testName, testTags: _*)(testFun)
+  }
+
+  protected def withNamespaceAndTable(ns: String, tableName: String, cat: String = catalog)


very nit: the parameter names for namespace and catalog are both very short(ns and cat), we should probably use t to be consistent.

If the build fails, I will change this to re-trigger GA or Jenkins.

SparkQA · 2020-12-15T14:16:36Z

Test build #132834 has started for PR 30779 at commit 04fd90b.

SparkQA · 2020-12-15T14:23:02Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37430/

SparkQA · 2020-12-15T15:03:35Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37436/

SparkQA · 2020-12-15T15:31:55Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37436/

SparkQA · 2020-12-15T17:31:43Z

Test build #132828 has finished for PR 30779 at commit 5836e54.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait AlterTableDropPartitionSuiteBase extends QueryTest with DDLCommandTestUtils

…ified-tests # Conflicts: # sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala # sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterTableDropPartitionSuite.scala # sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterTableDropPartitionSuite.scala # sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterTableDropPartitionSuite.scala

cloud-fan · 2020-12-16T05:47:40Z

@MaxGekk can you fix the conflicts?

SparkQA · 2020-12-16T06:53:34Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37462/

SparkQA · 2020-12-16T07:23:01Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37462/

SparkQA · 2020-12-16T10:28:46Z

Test build #132860 has finished for PR 30779 at commit 35dc058.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class SubqueryExec(name: String, child: SparkPlan, maxNumRows: Option[Int] = None)

cloud-fan · 2020-12-16T13:49:44Z

thanks, merging to master!

MaxGekk added 6 commits December 15, 2020 13:01

Move a test to DDLCommandTestUtils

42d15f5

Move withNsTable() to DDLCommandTestUtils

e699ab8

Re-use withNsTable in SHOW TABLES and SHOW PARTITIONS tests

853f0fe

Move checkPartitions() to DDLCommandTestUtils

4a0ded4

Put common code to CommandSuiteBase

99da3cd

Remove unused imports

89e3898

github-actions bot added the SQL label Dec 15, 2020

MaxGekk mentioned this pull request Dec 15, 2020

[SPARK-33767][SQL][TESTS] Unify v1 and v2 ALTER TABLE .. DROP PARTITION tests #30747

Closed

jbampton reviewed Dec 15, 2020

View reviewed changes

...src/test/scala/org/apache/spark/sql/execution/command/AlterTableDropPartitionSuiteBase.scala Outdated Show resolved Hide resolved

Update sql/core/src/test/scala/org/apache/spark/sql/execution/command…

5836e54

…/AlterTableDropPartitionSuiteBase.scala Co-authored-by: John Bampton <jbampton@users.noreply.github.com>

cloud-fan reviewed Dec 15, 2020

View reviewed changes

MaxGekk added 2 commits December 15, 2020 17:01

Remove some methods

838fb00

withNsTable -> withNamespaceAndTable

04fd90b

cloud-fan reviewed Dec 15, 2020

View reviewed changes

cloud-fan approved these changes Dec 15, 2020

View reviewed changes

cloud-fan closed this in 9d9d4a8 Dec 16, 2020

MaxGekk deleted the refactor-unified-tests branch February 19, 2021 15:04

[SPARK-33789][SQL][TESTS] Refactor unified V1 and V2 datasource tests #30779

[SPARK-33789][SQL][TESTS] Refactor unified V1 and V2 datasource tests #30779

Uh oh!

Conversation

MaxGekk commented Dec 15, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

SparkQA commented Dec 15, 2020

Uh oh!

cloud-fan commented Dec 16, 2020

Uh oh!

SparkQA commented Dec 16, 2020

Uh oh!

SparkQA commented Dec 16, 2020

Uh oh!

SparkQA commented Dec 16, 2020

Uh oh!

cloud-fan commented Dec 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants