You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/07/25 03:08:26 UTC
[GitHub] [spark] beliefer opened a new pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
beliefer opened a new pull request #29228:
URL: https://github.com/apache/spark/pull/29228
### 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678947156
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127831/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663799898
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685390658
**[Test build #128190 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128190/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676850514
**[Test build #127669 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127669/testReport)** for PR 29228 at commit [`b258649`](https://github.com/apache/spark/commit/b258649150b53f543779428672a8c482faeab4fb).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r473777248
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
+ with BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+
+ private var _conf: SparkConf = new SparkConf()
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning("Because SparkContext already initialized, " +
+ "since configurations won't take effect in this case.")
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686284767
**[Test build #128234 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128234/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677495574
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474404603
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -235,7 +235,7 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
_endedTasks.toSet
}
- private def waitForListeners(): Unit = sc.listenerBus.waitUntilEmpty()
+ private def waitForListeners(): Unit = sparkCtx.listenerBus.waitUntilEmpty()
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685378182
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689895140
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677530739
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683306071
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676850725
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678888647
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247988
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. ~We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.~(Sorry, just realized that it's actually the same with current implementation)
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
class DAGSchedulerSuite extends LocalSparkContext ... {
private var firstInit: Boolean = _
override def beforeEach: Unit = {
super.beforeEach()
firstInit = true
}
override def sc: SparkContext = {
super.sc()
if (firstInit) {
init(sc)
firstInit = false
}
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685367236
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686331439
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r479660053
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/**
+ * Manages a local `sc` `SparkContext` variable, correctly stopping it after each test.
+ *
+ * Note: this class is a copy of [[LocalSparkContext]]. Why copy it? Reduce conflict. Because
+ * many test suites use [[LocalSparkContext]] and overwrite some variable or function (e.g.
+ * sc of LocalSparkContext), there occurs conflict when we refactor the `sc` as a new function.
+ * After migrating all test suites that use [[LocalSparkContext]] to use [[LocalSC]], we will
+ * delete the original [[LocalSparkContext]] and rename [[LocalSC]] to [[LocalSparkContext]].
+ */
+trait LocalSC extends BeforeAndAfterEach
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272018
**[Test build #128231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128231/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685390783
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128190/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676949986
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676850514
**[Test build #127669 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127669/testReport)** for PR 29228 at commit [`b258649`](https://github.com/apache/spark/commit/b258649150b53f543779428672a8c482faeab4fb).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r472402621
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
+1 to the above proposal
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663802397
**[Test build #126524 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126524/testReport)** for PR 29228 at commit [`acb4e80`](https://github.com/apache/spark/commit/acb4e808b0f2e2ca0c0c72a7aa6f1378d10da62f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800430
**[Test build #126523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126523/testReport)** for PR 29228 at commit [`14de6c2`](https://github.com/apache/spark/commit/14de6c264c182a084300ed24046c25509e587b83).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663815547
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685376096
**[Test build #128190 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128190/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678967717
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678221946
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678937462
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r460627760
##########
File path: core/src/test/scala/org/apache/spark/SparkConfHelper.scala
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark
+
+trait SparkConfHelper {
+
+ /** Sets all configurations specified in `pairs`, calls `init`, and then calls `testFun` */
+ protected def withSparkConf(pairs: (String, String)*)(testFun: SparkConf => Any): Unit = {
Review comment:
`SparkConfHelper.withSparkConf` seems only used `DAGSchedulerSuite`. Let's don't make a trait for now but add it into `DAGSchedulerSuite`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663802552
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678089314
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677997923
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474393092
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
```
[error] /home/jenkins/workspace/SparkPullRequestBuilder@2/resource-managers/mesos/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala:49: overriding method sparkConf in trait LocalSparkContext of type => org.apache.spark.SparkConf;
[error] variable sparkConf has weaker access privileges; it should not be private
[error] private var sparkConf: SparkConf = _
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272672
**[Test build #128231 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128231/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686285504
**[Test build #128234 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128234/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch **fails RAT tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686419138
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663802552
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474386172
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
nit: `def conf`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474417075
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -1104,7 +1104,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleOneRdd = new MyRDD(sc, 2, Nil).cache()
val shuffleDepOne = new ShuffleDependency(shuffleOneRdd, new HashPartitioner(2))
- val shuffleTwoRdd = new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
+ val shuffleTwoRdd =
+ new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
Review comment:
unnecessary change?
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -1153,7 +1154,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleOneRdd = new MyRDD(sc, 2, Nil).cache()
val shuffleDepOne = new ShuffleDependency(shuffleOneRdd, new HashPartitioner(2))
- val shuffleTwoRdd = new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
+ val shuffleTwoRdd =
+ new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -1757,7 +1759,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleDep2 = new ShuffleDependency(shuffleMapRdd2, new HashPartitioner(2))
val reduceRdd1 = new MyRDD(sc, 2, List(shuffleDep1), tracker = mapOutputTracker)
- val reduceRdd2 = new MyRDD(sc, 2, List(shuffleDep1, shuffleDep2), tracker = mapOutputTracker)
+ val reduceRdd2 =
+ new MyRDD(sc, 2, List(shuffleDep1, shuffleDep2), tracker = mapOutputTracker)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -1954,7 +1957,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
test("cached post-shuffle") {
val shuffleOneRdd = new MyRDD(sc, 2, Nil).cache()
val shuffleDepOne = new ShuffleDependency(shuffleOneRdd, new HashPartitioner(2))
- val shuffleTwoRdd = new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
+ val shuffleTwoRdd =
+ new MyRDD(sc, 2, List(shuffleDepOne), tracker = mapOutputTracker).cache()
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678936184
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678923619
**[Test build #127827 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127827/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678033737
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677997923
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474416962
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -460,7 +470,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleDepB = new ShuffleDependency(rddB, new HashPartitioner(1))
val s_B = shuffleDepB.shuffleId
- val rddC = new MyRDD(sc, 1, List(shuffleDepA, shuffleDepB), tracker = mapOutputTracker)
+ val rddC =
+ new MyRDD(sc, 1, List(shuffleDepA, shuffleDepB), tracker = mapOutputTracker)
Review comment:
unnecessary change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-679047370
**[Test build #127837 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127837/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678221946
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676948299
**[Test build #127672 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127672/testReport)** for PR 29228 at commit [`69790ad`](https://github.com/apache/spark/commit/69790adb810b5ca2f414f881b36d34e0bdc201a5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272694
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474386809
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -235,7 +235,7 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
_endedTasks.toSet
}
- private def waitForListeners(): Unit = sc.listenerBus.waitUntilEmpty()
+ private def waitForListeners(): Unit = sparkCtx.listenerBus.waitUntilEmpty()
Review comment:
I'm thinking if we could have a new copy of `LocalSparkContext`, e.g., `LocalSC`, then, we don't need to change from `sc` to `sparkCtx`? And for other test suites, we only need to replace `LocalSparkContext` with `LocalSC`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678937036
**[Test build #127831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127831/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474393848
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning(s"These configurations ${pairs.mkString(", ")} won't take effect" +
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247984
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686417914
**[Test build #128244 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128244/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677996115
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r473780769
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
+ with BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+
+ private var _conf: SparkConf = new SparkConf()
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689270837
cc @jiangxb1987
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676850725
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676136380
**[Test build #127645 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127645/testReport)** for PR 29228 at commit [`a1fb471`](https://github.com/apache/spark/commit/a1fb471b6b49aeba8a0581a61ca5e4d9f76387a0).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #29228:
URL: https://github.com/apache/spark/pull/29228
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689833873
**[Test build #128470 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128470/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678888372
**[Test build #127827 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127827/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474417248
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -2255,7 +2259,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleDep = new ShuffleDependency(rdd1, new HashPartitioner(1))
val narrowDep = new OneToOneDependency(rdd2)
val shuffleId = shuffleDep.shuffleId
- val reduceRdd = new MyRDD(sc, 1, List(shuffleDep, narrowDep), tracker = mapOutputTracker)
+ val reduceRdd =
+ new MyRDD(sc, 1, List(shuffleDep, narrowDep), tracker = mapOutputTracker)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -2851,11 +2856,13 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleDep1 = new ShuffleDependency(shuffleMapRdd1, new HashPartitioner(2))
val shuffleId1 = shuffleDep1.shuffleId
- val shuffleMapRdd2 = new MyRDD(sc, 2, List(shuffleDep1), tracker = mapOutputTracker)
+ val shuffleMapRdd2 =
+ new MyRDD(sc, 2, List(shuffleDep1), tracker = mapOutputTracker)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -2851,11 +2856,13 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val shuffleDep1 = new ShuffleDependency(shuffleMapRdd1, new HashPartitioner(2))
val shuffleId1 = shuffleDep1.shuffleId
- val shuffleMapRdd2 = new MyRDD(sc, 2, List(shuffleDep1), tracker = mapOutputTracker)
+ val shuffleMapRdd2 =
+ new MyRDD(sc, 2, List(shuffleDep1), tracker = mapOutputTracker)
val shuffleDep2 = new ShuffleDependency(shuffleMapRdd2, new HashPartitioner(2))
val shuffleId2 = shuffleDep2.shuffleId
- val finalRdd = new MyRDD(sc, 2, List(shuffleDep2), tracker = mapOutputTracker)
+ val finalRdd =
+ new MyRDD(sc, 2, List(shuffleDep2), tracker = mapOutputTracker)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -3198,7 +3201,8 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val treqs2 = new TaskResourceRequests().cpus(2)
val rp2 = new ResourceProfileBuilder().require(ereqs2).require(treqs2).build
- val rdd = sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
+ val rdd =
+ sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -3221,18 +3222,16 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val treqs2 = new TaskResourceRequests().cpus(2)
val rp2 = new ResourceProfileBuilder().require(ereqs2).require(treqs2).build
- val rdd = sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
+ val rdd =
+ sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686284767
**[Test build #128234 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128234/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800601
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678937462
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677689536
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127690/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663799816
cc @jiangxb1987 @Ngone51
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689834426
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r473814481
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,11 +295,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ firstInit = true
+ setConf("spark.master" -> "local[2]", "spark.app.name" -> "DAGSchedulerSuite")
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678924208
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800599
**[Test build #126523 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126523/testReport)** for PR 29228 at commit [`14de6c2`](https://github.com/apache/spark/commit/14de6c264c182a084300ed24046c25509e587b83).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait SparkConfHelper `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677529870
**[Test build #127690 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127690/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678937036
**[Test build #127831 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127831/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663802397
**[Test build #126524 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126524/testReport)** for PR 29228 at commit [`acb4e80`](https://github.com/apache/spark/commit/acb4e808b0f2e2ca0c0c72a7aa6f1378d10da62f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r472594156
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
OK. I will implement this proposal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474427826
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678888372
**[Test build #127827 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127827/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676164984
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800430
**[Test build #126523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126523/testReport)** for PR 29228 at commit [`14de6c2`](https://github.com/apache/spark/commit/14de6c264c182a084300ed24046c25509e587b83).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678035815
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683237385
LGTM otherwise
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685390755
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081216
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677491444
**[Test build #127686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127686/testReport)** for PR 29228 at commit [`bd9b85f`](https://github.com/apache/spark/commit/bd9b85fbebdf8da2889ef1abe7e52c6ae4d60f60).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800607
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/126523/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474435459
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
Then, let's use `def conf: SparkConf`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676852591
**[Test build #127669 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127669/testReport)** for PR 29228 at commit [`b258649`](https://github.com/apache/spark/commit/b258649150b53f543779428672a8c482faeab4fb).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678089570
**[Test build #127732 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127732/testReport)** for PR 29228 at commit [`c7e5e7b`](https://github.com/apache/spark/commit/c7e5e7b6755eead2bf6c96c6b8fe43e4e09d4f23).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685390755
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678947071
**[Test build #127831 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127831/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686285525
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128234/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676164984
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677492055
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683306071
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678089570
**[Test build #127732 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127732/testReport)** for PR 29228 at commit [`c7e5e7b`](https://github.com/apache/spark/commit/c7e5e7b6755eead2bf6c96c6b8fe43e4e09d4f23).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474434857
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
No
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689833873
**[Test build #128470 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128470/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678970522
**[Test build #127837 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127837/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678947146
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663799898
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272694
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247988
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. ~We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.~(Sorry, just realized that it's actually the same with current implementation)
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
class DAGSchedulerSuite extends LocalSparkContext ... {
private var firstInit: Boolean = _
override def beforeEach: Unit = {
super.beforeEach()
firstInit = true
}
override def sc: SparkContext = {
val sc = super.sc()
if (firstInit) {
init(sc)
firstInit = false
}
sc
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663800601
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247988
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
class DAGSchedulerSuite extends LocalSparkContext ... {
private var firstInit: Boolean = _
override def beforeEach: Unit = {
super.beforeEach()
firstInit = true
}
override def sc: SparkContext = {
super.sc()
if (firstInit) {
init(sc)
firstInit = false
}
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677491444
**[Test build #127686 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127686/testReport)** for PR 29228 at commit [`bd9b85f`](https://github.com/apache/spark/commit/bd9b85fbebdf8da2889ef1abe7e52c6ae4d60f60).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676852606
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676949986
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663815399
**[Test build #126524 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/126524/testReport)** for PR 29228 at commit [`acb4e80`](https://github.com/apache/spark/commit/acb4e808b0f2e2ca0c0c72a7aa6f1378d10da62f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686330659
**[Test build #128244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128244/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686330181
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677495574
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677495581
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127686/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081211
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678069340
**[Test build #127725 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127725/testReport)** for PR 29228 at commit [`c7e5e7b`](https://github.com/apache/spark/commit/c7e5e7b6755eead2bf6c96c6b8fe43e4e09d4f23).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474417730
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
Review comment:
Could you explain more about why we need this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676165006
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127646/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689832690
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689895140
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474396405
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
Review comment:
`setConf` just sounds like an attribute setting function which needs to invoke like `conf.setConf`. After we exposing the `conf`, I think it's more straightforward to use it like `conf.setConf(...)` or `conf.setAll(...)`. Here, I think what we want is a helper method, e.g., `withConf`. And `withXXX` is more like a common pattern in Spark. We can use it to work with other helper methods like, `withTempDir`, `withListener`, and others in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686271317
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676948299
**[Test build #127672 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127672/testReport)** for PR 29228 at commit [`69790ad`](https://github.com/apache/spark/commit/69790adb810b5ca2f414f881b36d34e0bdc201a5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677239903
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677239903
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686282242
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685378182
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r467539295
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
If I understand correctly, you are doing the hack here because you need to modify the SparkConf within `beforeEach` (between `super.beforeEach()` and `init()`). In other words, you don't need to call `testWithSparkConf` instead of `test`, if you don't do extra initialization at the `beforeEach()` stage. Thus, this change is necessarily useful for those test suites you listed, right?
A more staightforward idea would likely to be having a `withConfig(pairs: (String, String)*)` method to create a new SparkConf with the specified configuration values？ The idea doesn't simplify `DAGSchedulerSuite` that much, because you still need to first stop the SparkContext then call `init()` with your own SparkConf, but at least it's not worse than the current approach, and more easy to understand and to be reused.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-663815547
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676136458
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r473776284
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676143482
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r473734385
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
+ with BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+
+ private var _conf: SparkConf = new SparkConf()
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning("Because SparkContext already initialized, " +
+ "since configurations won't take effect in this case.")
Review comment:
nit: `These configurations ${paris.mkString(", ")} won't take effect since the SparkContext has been already initialized.`
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
Review comment:
I think we usually put `Logging` at the end.
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,37 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends Logging
+ with BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+
+ private var _conf: SparkConf = new SparkConf()
Review comment:
I think `SparkConf` should have the default values for `master` and `appName`. So test suites extend it could use the SparkContext directly without any specific configurations when the test doesn't really care.
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,11 +295,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ firstInit = true
+ setConf("spark.master" -> "local[2]", "spark.app.name" -> "DAGSchedulerSuite")
Review comment:
We'd better to expose the `conf` via a function to set the conf. `setConf` is designed to be used by the test only.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678924209
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127827/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678033375
**[Test build #127720 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127720/testReport)** for PR 29228 at commit [`009add9`](https://github.com/apache/spark/commit/009add95a1d740ad1712f302c97970df26bc8390).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474434997
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sc: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677492055
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678033737
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247988
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
init(_sc)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
class DAGSchedulerSuite extends LocalSparkContext ... {
private var firstInit: Boolean = _
override def beforeEach: Unit = {
super.beforeEach()
firstInit = true
}
override def sc: SparkContext = {
super.sc()
if (firstInit) {
init(sc)
firstInit = false
}
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683323517
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686285516
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-666137629
It would be really great if you can list the test cases/suites that could get simplified by this change, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r467661784
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
For your first question, yes, it is.
For your second question, `withConfig(pairs: (String, String)*)` mean to stop the `SparkContext` and then call `init()`. So I created the function `testWithSparkConf` which avoid to stop the `SparkContext` then call `init()`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677529870
**[Test build #127690 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127690/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677689531
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-685376096
**[Test build #128190 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128190/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676164878
**[Test build #127646 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127646/testReport)** for PR 29228 at commit [`c95f004`](https://github.com/apache/spark/commit/c95f0047e6aa0dcbd3df1de3cb20ca3bd3e3cc0d).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474418302
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
Would `def conf: SparkConf` still cause the compile error after we moving all the things to a separate file?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683305936
**[Test build #128018 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128018/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683323329
**[Test build #128018 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128018/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247984
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
```
@beliefer @jiangxb1987 WDYT?
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686419138
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474419910
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sc: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning(s"These configurations ${pairs.mkString(", ")} won't take effect " +
+ "since the SparkContext has been already initialized.")
+ }
+ pairs.foreach { case (k, v) => _conf.set(k, v) }
+ }
+
+ override def beforeAll(): Unit = {
+ super.beforeAll()
+ InternalLoggerFactory.setDefaultFactory(Slf4JLoggerFactory.INSTANCE)
+ }
+
+ override def afterEach(): Unit = {
+ try {
+ resetSparkContext()
+ } finally {
+ super.afterEach()
+ }
+ }
+
+ def resetSparkContext(): Unit = {
+ LocalSC.stop(_sc)
+ ResourceProfile.clearDefaultProfile()
+ _sc = null
+ _conf = defaultSparkConf
+ }
+
+ private def defaultSparkConf: SparkConf = new SparkConf()
+ .setMaster("local[2]").setAppName(s"${this.getClass.getSimpleName}")
+
+}
+
+object LocalSC {
+ def stop(sc: SparkContext): Unit = {
+ if (sc != null) {
+ sc.stop()
+ }
+ // To avoid RPC rebinding to the same port, since it doesn't unbind immediately on shutdown
+ System.clearProperty("spark.driver.port")
+ }
+
+ /** Runs `f` by passing in `sc` and ensures that `sc` is stopped. */
+ def withSpark[T](sc: SparkContext)(f: SparkContext => T): T = {
+ try {
+ f(sc)
+ } finally {
+ stop(sc)
+ }
+ }
+
Review comment:
nit: no blank line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683305936
**[Test build #128018 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128018/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676852606
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081110
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-683323517
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678970978
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r479608346
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/**
+ * Manages a local `sc` `SparkContext` variable, correctly stopping it after each test.
+ *
+ * Note: this class is a copy of [[LocalSparkContext]]. Why copy it? Reduce conflict. Because
+ * many test suites use [[LocalSparkContext]] and overwrite some variable or function (e.g.
+ * sc of LocalSparkContext), there occurs conflict when we refactor the `sc` as a new function.
+ * After migrating all test suites that use [[LocalSparkContext]] to use [[LocalSC]], we will
+ * delete the original [[LocalSparkContext]] and rename [[LocalSC]] to [[LocalSparkContext]].
+ */
+trait LocalSC extends BeforeAndAfterEach
Review comment:
Since this class is only used for temporary purpose, can we name it as `TempLocalSparkContext` ? TBH I don't like the `SC` name which is very vague to me.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676132441
**[Test build #127645 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127645/testReport)** for PR 29228 at commit [`a1fb471`](https://github.com/apache/spark/commit/a1fb471b6b49aeba8a0581a61ca5e4d9f76387a0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474392815
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
@transient var sc: SparkContext = _
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sparkCtx: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning(s"These configurations ${pairs.mkString(", ")} won't take effect" +
Review comment:
nit: need a space after "effect"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678090060
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677530739
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474419836
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sc: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
+ if (_sc != null) {
+ logWarning(s"These configurations ${pairs.mkString(", ")} won't take effect " +
+ "since the SparkContext has been already initialized.")
+ }
+ pairs.foreach { case (k, v) => _conf.set(k, v) }
+ }
+
+ override def beforeAll(): Unit = {
+ super.beforeAll()
+ InternalLoggerFactory.setDefaultFactory(Slf4JLoggerFactory.INSTANCE)
+ }
+
+ override def afterEach(): Unit = {
+ try {
+ resetSparkContext()
+ } finally {
+ super.afterEach()
+ }
+ }
+
+ def resetSparkContext(): Unit = {
+ LocalSC.stop(_sc)
+ ResourceProfile.clearDefaultProfile()
+ _sc = null
+ _conf = defaultSparkConf
+ }
+
+ private def defaultSparkConf: SparkConf = new SparkConf()
+ .setMaster("local[2]").setAppName(s"${this.getClass.getSimpleName}")
+
Review comment:
nit: no blank line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081893
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127720/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678217743
**[Test build #127732 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127732/testReport)** for PR 29228 at commit [`c7e5e7b`](https://github.com/apache/spark/commit/c7e5e7b6755eead2bf6c96c6b8fe43e4e09d4f23).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-679048530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677688468
**[Test build #127690 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127690/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676132441
**[Test build #127645 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127645/testReport)** for PR 29228 at commit [`a1fb471`](https://github.com/apache/spark/commit/a1fb471b6b49aeba8a0581a61ca5e4d9f76387a0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676124666
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-679048530
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686330659
**[Test build #128244 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128244/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474657779
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/**
+ * Manages a local `sc` `SparkContext` variable, correctly stopping it after each test.
+ *
+ * Note: this class is a copy of [[LocalSparkContext]]. Why copy it? Reduce conflict. Because
+ * many test suites use [[LocalSparkContext]] and overwrite some variable or function (e.g.
+ * sc of LocalSparkContext), there occurs conflict when we refactor the `sc` as a new function.
+ * After migrating all test suites that use [[LocalSparkContext]] to use [[LocalSC]], we will
+ * delete the original [[LocalSparkContext]] and rename [[LocalSC]] to [[LocalSparkContext]].
+ */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def conf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sc: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def withConf(pairs: (String, String)*): Unit = {
Review comment:
Maybe like this?
```scala
def withConf(pairs: (String, String)*)(f: => Unit) = {
try f finally {
// reset conf here
}
}
```
If so, we don't need to create the new SparkConf for each test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676151941
**[Test build #127646 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127646/testReport)** for PR 29228 at commit [`c95f004`](https://github.com/apache/spark/commit/c95f0047e6aa0dcbd3df1de3cb20ca3bd3e3cc0d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678947146
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081887
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678090060
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678035815
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474418486
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
+
+ /**
+ * Currently, we are focusing on the reconstruction of LocalSparkContext, so this method
+ * was created temporarily. When the migration work is completed, this method will be
+ * renamed to `sc` and the variable `sc` will be deleted.
+ */
+ def sc: SparkContext = {
+ if (_sc == null) {
+ _sc = new SparkContext(_conf)
+ }
+ _sc
+ }
+
+ def setConf(pairs: (String, String)*): Unit = {
Review comment:
In case you miss this comment: https://github.com/apache/spark/pull/29228#discussion_r474396405
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272469
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677997610
**[Test build #127713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127713/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474417730
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
Review comment:
Could explain more about why we need this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677689531
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677235974
**[Test build #127672 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127672/testReport)** for PR 29228 at commit [`69790ad`](https://github.com/apache/spark/commit/69790adb810b5ca2f414f881b36d34e0bdc201a5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676151941
**[Test build #127646 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127646/testReport)** for PR 29228 at commit [`c95f004`](https://github.com/apache/spark/commit/c95f0047e6aa0dcbd3df1de3cb20ca3bd3e3cc0d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678069823
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689894344
**[Test build #128470 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128470/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DAGSchedulerSuite extends SparkFunSuite with TempLocalSparkContext with TimeLimits `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678924208
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676143482
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676136458
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272700
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128231/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676124666
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474437466
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging { self: Suite =>
+
+ private var _conf: SparkConf = defaultSparkConf
+
+ @transient private var _sc: SparkContext = _
+
+ def getSparkConf: SparkConf = _conf
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677997610
**[Test build #127713 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127713/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686331439
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272469
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686272018
**[Test build #128231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/128231/testReport)** for PR 29228 at commit [`1029d26`](https://github.com/apache/spark/commit/1029d2658fa18cfa773fef5ede283a3e7184f438).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678081211
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r460640617
##########
File path: core/src/test/scala/org/apache/spark/SparkConfHelper.scala
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.spark
+
+trait SparkConfHelper {
+
+ /** Sets all configurations specified in `pairs`, calls `init`, and then calls `testFun` */
+ protected def withSparkConf(pairs: (String, String)*)(testFun: SparkConf => Any): Unit = {
Review comment:
`SparkConfHelper.withSparkConf` will be used in other suite like `TaskSchedulerImplSuite`、`TaskSetManagerSuite`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-680417694
@Ngone51 I updated the description of this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678970978
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678888647
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678970522
**[Test build #127837 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127837/testReport)** for PR 29228 at commit [`86dc8f8`](https://github.com/apache/spark/commit/86dc8f81702f6694bd17d4578d81133ce0731ac5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474386042
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
Review comment:
nit: needs a space before `{`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676136487
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127645/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678069823
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474417504
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -3242,14 +3241,16 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val treqs2 = new TaskResourceRequests().cpus(2)
val rp2 = new ResourceProfileBuilder().require(ereqs2).require(treqs2).build
- val rdd = sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
+ val rdd = sc.parallelize(1 to 10)
+ .withResources(rp1).map(x => (x, x)).withResources(rp2)
Review comment:
ditto
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -3242,14 +3241,16 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
val treqs2 = new TaskResourceRequests().cpus(2)
val rp2 = new ResourceProfileBuilder().require(ereqs2).require(treqs2).build
- val rdd = sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
+ val rdd = sc.parallelize(1 to 10)
+ .withResources(rp1).map(x => (x, x)).withResources(rp2)
val (_, resourceprofiles) = scheduler.getShuffleDependenciesAndResourceProfiles(rdd)
val mergedRp = scheduler.mergeResourceProfilesForStage(resourceprofiles)
assert(mergedRp.getTaskCpus.get == 2)
assert(mergedRp.getExecutorCores.get == 4)
// test that instead of creating a new merged profile, we use the already created one
- val rdd2 = sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
+ val rdd2 =
+ sc.parallelize(1 to 10).withResources(rp1).map(x => (x, x)).withResources(rp2)
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-677495543
**[Test build #127686 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127686/testReport)** for PR 29228 at commit [`bd9b85f`](https://github.com/apache/spark/commit/bd9b85fbebdf8da2889ef1abe7e52c6ae4d60f60).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678035333
**[Test build #127713 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/127713/testReport)** for PR 29228 at commit [`4001780`](https://github.com/apache/spark/commit/4001780ac6e4366b4eaace2bebc45bbde65e8939).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-678033375
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-676852615
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/127669/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474392606
##########
File path: core/src/test/scala/org/apache/spark/LocalSparkContext.scala
##########
@@ -22,12 +22,39 @@ import org.scalatest.BeforeAndAfterAll
import org.scalatest.BeforeAndAfterEach
import org.scalatest.Suite
+import org.apache.spark.internal.Logging
import org.apache.spark.resource.ResourceProfile
/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
-trait LocalSparkContext extends BeforeAndAfterEach with BeforeAndAfterAll { self: Suite =>
+trait LocalSparkContext extends BeforeAndAfterEach
+ with BeforeAndAfterAll with Logging{ self: Suite =>
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
beliefer commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r474436970
##########
File path: core/src/test/scala/org/apache/spark/LocalSC.scala
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark
+
+import _root_.io.netty.util.internal.logging.{InternalLoggerFactory, Slf4JLoggerFactory}
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.Suite
+
+import org.apache.spark.internal.Logging
+import org.apache.spark.resource.ResourceProfile
+
+/** Manages a local `sc` `SparkContext` variable, correctly stopping it after each test. */
+trait LocalSC extends BeforeAndAfterEach
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686285330
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-680006930
LGTM. BTW, could you update the PR description? You may not need to list all the test suites with the latest change but just say test suites inherits `LocalSparkContext` can be simplified.
@jiangxb1987 Could you also take a look?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-686285330
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #29228:
URL: https://github.com/apache/spark/pull/29228#discussion_r471247988
##########
File path: core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala
##########
@@ -295,7 +298,20 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi
override def beforeEach(): Unit = {
super.beforeEach()
- init(new SparkConf())
+ }
+
+ override protected def test(testName: String, testTags: Tag*)(testFun: => Any)
+ (implicit pos: Position): Unit = {
+ testWithSparkConf(testName, testTags: _*)()(testFun)(pos)
+ }
+
+ private def testWithSparkConf(testName: String, testTags: Tag*)
Review comment:
I tend to agree with @jiangxb1987 . There're only 7 places using the `testWithSparkConf()` comparing to other 81 tests within the `DAGSchedulerSuite`. We could just call `init` inside each test to ease other tests who needs to call `afterEach` inside the `test`.
And I have another idea for the whole thing. That is, we could probably initialize the `SparkContext` like this:
```scala
trait LocalSparkContext ... {
@transient private var _sc: SparkContext = _
private val conf = new SparkConf()
def sc: SparkContext = {
if (_sc == null) {
_sc = new SparkContext(conf)
init(_sc)
}
_sc
}
def withConf(pairs: (String, String)*) = {
if (_sc != null) {
// probably, log warning when SparkContext already initialized
// since configurations won't take effect in this case
}
paris.foreach { case (k, v) => conf.set(k, v) }
}
override def afterEach(): Unit = {
try {
resetSparkContext()
} finally {
super.afterEach()
}
}
def resetSparkContext(): Unit = {
LocalSparkContext.stop(sc)
ResourceProfile.clearDefaultProfile()
sc = null
}
}
class DAGSchedulerSuite extends LocalSparkContext ... {
private var firstInit: Boolean = _
override def beforeEach: Unit = {
super.beforeEach()
firstInit = false
}
override def sc: SparkContext = {
super.sc()
if (!firstInit) {
init(sc)
firstInit = true
}
}
}
```
@beliefer @jiangxb1987 WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-689834426
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #29228: [SPARK-31847][CORE][TESTS] DAGSchedulerSuite: Rewrite the test framework to support apply specified spark configurations.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29228:
URL: https://github.com/apache/spark/pull/29228#issuecomment-691782280
Merged to master.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org