[SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations. by beliefer · Pull Request #28917 · apache/spark

beliefer · 2020-06-24T07:48:26Z

What changes were proposed in this pull request?

DAGSchedulerSuite exists some issue:
afterEach and init are called when the SparkConf of the default SparkContext has no configuration that the test case must set. This causes the SparkContext initialized in beforeEach to be discarded without being used, resulting in waste. On the other hand, the flexibility to add configurations to SparkConf should be addressed by the test framework.

Why are the changes needed?

Reduce overhead about init SparkContext.
Rewrite the test framework to support apply specified spark configurations.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Jenkins test.

SparkQA · 2020-06-24T07:53:39Z

Test build #124472 has finished for PR 28917 at commit a1c3bb8.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-06-24T11:02:58Z

core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala

+    testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+  }
+
+  protected def testWithSparkConf(testName: String, testTags: Tag*)


Seems like we just need testWithSparkConf alone. Can we just have this as a private function alone?

SparkQA · 2020-06-24T11:03:23Z

Test build #124474 has finished for PR 28917 at commit 4102f02.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-28T06:33:56Z

Test build #124587 has finished for PR 28917 at commit 350fa8d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-06-29T01:42:31Z

core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala

+    testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+  }
+
+  private def testWithSparkConf(testName: String, testTags: Tag*)


Shall we simulate the usage of withSQLConf instead of integrating the confs with test()?

Most SQL-related configuration parameters can be changed dynamically, but most of Core's parameters are static.

Ngone51 · 2020-06-29T01:44:04Z

core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala

+  }
+
+  /** Sets all configurations specified in `pairs`, calls `init`, and then calls `testFun` */
+  private def withSparkConf(pairs: (String, String)*)(testFun: => Any): Unit = {


For test(), withSparkConf(), shall we extract them into a base class? I guess they could be used by other test suites as well?

OK. I will simulate SQLHelper.

Ngone51 · 2020-06-29T01:47:29Z

core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala

+  private def withSparkConf(pairs: (String, String)*)(testFun: => Any): Unit = {
+    val conf = new SparkConf()
+    pairs.foreach(kv => conf.set(kv._1, kv._2))
+    init(conf)


init() is specific to DAGSchedulerSuite, we should separate it from the test framework.

SparkQA · 2020-06-30T16:26:42Z

Test build #124661 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-01T01:55:48Z

retest this please.

SparkQA · 2020-07-01T02:03:38Z

Test build #124723 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-01T02:41:36Z

retest this please.

SparkQA · 2020-07-01T02:47:19Z

Test build #124730 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-01T03:51:26Z

retest this please.

SparkQA · 2020-07-01T07:37:41Z

Test build #124738 has finished for PR 28917 at commit da6a9f7.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-01T07:44:16Z

retest this please.

SparkQA · 2020-07-01T11:22:49Z

Test build #124756 has finished for PR 28917 at commit da6a9f7.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-02T01:53:28Z

retest this please.

SparkQA · 2020-07-02T02:01:13Z

Test build #124841 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-02T02:57:35Z

retest this please.

SparkQA · 2020-07-02T03:02:11Z

Test build #124851 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-02T03:11:49Z

retest this please.

SparkQA · 2020-07-02T03:17:19Z

Test build #124852 has finished for PR 28917 at commit da6a9f7.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-02T03:20:11Z

retest this please.

SparkQA · 2020-07-02T06:40:59Z

Test build #124854 has finished for PR 28917 at commit da6a9f7.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
trait SparkConfHelper

beliefer · 2020-07-14T06:41:52Z

cc @jiangxb1987

SparkQA · 2020-07-14T07:05:02Z

Test build #125808 has finished for PR 28917 at commit ec0d8d0.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2020-07-14T07:49:07Z

retest this please

SparkQA · 2020-07-14T11:32:13Z

Test build #125814 has finished for PR 28917 at commit ec0d8d0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-07-14T13:31:32Z

Test build #125822 has finished for PR 28917 at commit b060daa.

This patch fails PySpark pip packaging tests.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2020-07-14T14:24:06Z

retest this please

SparkQA · 2020-07-14T17:41:11Z

Test build #125842 has finished for PR 28917 at commit b060daa.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-07-15T21:36:31Z

Test build #125897 has finished for PR 28917 at commit ec0d8d0.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

beliefer · 2020-07-17T03:28:28Z

retest this please

SparkQA · 2020-07-17T07:05:02Z

Test build #126022 has finished for PR 28917 at commit ec0d8d0.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2020-07-17T07:21:30Z

retest this please

SparkQA · 2020-07-17T10:24:17Z

Test build #126041 has finished for PR 28917 at commit ec0d8d0.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

beliefer · 2020-07-21T02:09:23Z

cc @jiangxb1987 @Ngone51

beliefer · 2020-07-25T03:09:06Z

Because conflicts, I will close this PR.

beliefer added 2 commits June 19, 2020 10:36

Reuse completeNextStageWithFetchFailure

4a6f903

Extend test framework can sets the specified conf.

a1c3bb8

probot-autolabeler bot added the CORE label Jun 24, 2020

Fix scala style.

4102f02

HyukjinKwon reviewed Jun 24, 2020

View reviewed changes

Adjust code

350fa8d

Ngone51 reviewed Jun 29, 2020

View reviewed changes

Optimize code

da6a9f7

Merge branch 'master' into extend-test-frame

ec0d8d0

beliefer force-pushed the extend-test-frame branch from b060daa to ec0d8d0 Compare July 15, 2020 07:43

beliefer closed this Jul 25, 2020

Conversation

beliefer commented Jun 24, 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!

SparkQA commented Jun 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 24, 2020

Uh oh!

SparkQA commented Jun 28, 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!

SparkQA commented Jun 30, 2020

Uh oh!

beliefer commented Jul 1, 2020

Uh oh!

SparkQA commented Jul 1, 2020

Uh oh!

beliefer commented Jul 1, 2020

Uh oh!

SparkQA commented Jul 1, 2020

Uh oh!

beliefer commented Jul 1, 2020

Uh oh!

SparkQA commented Jul 1, 2020

Uh oh!

beliefer commented Jul 1, 2020

Uh oh!

SparkQA commented Jul 1, 2020

Uh oh!

beliefer commented Jul 2, 2020

Uh oh!

SparkQA commented Jul 2, 2020

Uh oh!

beliefer commented Jul 2, 2020

Uh oh!

SparkQA commented Jul 2, 2020

Uh oh!

beliefer commented Jul 2, 2020

Uh oh!

SparkQA commented Jul 2, 2020

Uh oh!

beliefer commented Jul 2, 2020

Uh oh!

SparkQA commented Jul 2, 2020

Uh oh!

beliefer commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 14, 2020

Uh oh!

beliefer commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 14, 2020

Uh oh!

beliefer commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 14, 2020

Uh oh!

SparkQA commented Jul 15, 2020

Uh oh!

beliefer commented Jul 17, 2020