[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2015-01-06 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68924192
  
Meh, let's just merge this in for now.  The appending behavior won't be 
confusing in Jenkins (since we clean) and I can only imagine this being 
annoying for developers who do incremental development in Maven on a local 
machine (probably a small minority of Spark developers).  Having the tests 
actually run is a higher priority than a minor developer-only usability issue 
that will affect a handful of users; we can address the append behavior in a 
separate followup PR if anyone complains.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2015-01-06 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68924468
  
Merged to `master` (1.3.0).  I'll handle the backport cherry-picks in a 
little bit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2015-01-06 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/3651


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2015-01-06 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68907313
  
@JoshRosen et al I'd love to get this in so that the Java test run again. 
Say the word and I'll add the extra clean configuration if that's desired. I 
think it's also OK as-is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-28 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68219056
  
@JoshRosen I think it's a small pain to add to the POM. You can bind a 
different execution of the Maven clean plugin to `test` or `test-compile`. I 
don't think it's hard, just more stuff in the POM. I had punted on it but if 
anyone votes for adding it I will do so.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-25 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68114368
  
 The only issue with using append = true is that multiple test invocations 
will just keep appending to the file, potentially making debugging a little 
more confusing. Not a big deal, I think. Maybe adding a task in the root pom 
that runs before surefire/scalatest and just deletes that file?

I suppose we still have this issue, right?  It might not be a big deal if 
people run `mvn clean` between builds, but I could imagine it being annoying if 
you're doing incremental re-builds during an iterative debugging session.  Is 
this hard to fix?  I don't think it's the end of the world if we don't get to 
this now, though.

Also, we should probably backport this change into the maintenance branches 
so that we can detect whether their Java tests are broken.  We should verify 
that `SPARK_HOME` is obsolete in yarn/repl for those versions, too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68039924
  
@JoshRosen I found that removing the `SPARK_HOME` config doesn't seem to 
matter; the REPL and YARN tests still pass. OK to remove that config in this 
PR, do you think? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68074101
  
I did a quick `git grep` through the codebase to find uses of `SPARK_HOME` 
and it looks like there's only a few places where it's read:

SparkContext, which is a fallback if `spark.home` is not set:

```
core/src/main/scala/org/apache/spark/SparkContext.scala-   * Get Spark's 
home location from either a value set through the constructor,
core/src/main/scala/org/apache/spark/SparkContext.scala-   * or the 
spark.home Java property, or the SPARK_HOME environment variable
core/src/main/scala/org/apache/spark/SparkContext.scala-   * (in that order 
of preference). If neither of these is set, return None.
core/src/main/scala/org/apache/spark/SparkContext.scala-   */
core/src/main/scala/org/apache/spark/SparkContext.scala-  private[spark] 
def getSparkHome(): Option[String] = {
core/src/main/scala/org/apache/spark/SparkContext.scala:
conf.getOption(spark.home).orElse(Option(System.getenv(SPARK_HOME)))
core/src/main/scala/org/apache/spark/SparkContext.scala-  }
core/src/main/scala/org/apache/spark/SparkContext.scala-
core/src/main/scala/org/apache/spark/SparkContext.scala-  /**
core/src/main/scala/org/apache/spark/SparkContext.scala-   * Set the 
thread-local property for overriding the call sites
core/src/main/scala/org/apache/spark/SparkContext.scala-   * of actions and 
RDDs.
```

PythonUtils, with no fallback:

```
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-

core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-private[spark]
 object PythonUtils {
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-  /** Get 
the PYTHONPATH for PySpark, either from SPARK_HOME, if it is set, or from our 
JAR */
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-  def 
sparkPythonPath: String = {
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-val 
pythonPath = new ArrayBuffer[String]
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala:for 
(sparkHome - sys.env.get(SPARK_HOME)) {
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-  
pythonPath += Seq(sparkHome, python).mkString(File.separator)
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-  
pythonPath += Seq(sparkHome, python, lib, 
py4j-0.8.2.1-src.zip).mkString(File.separator)
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-}
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-
pythonPath ++= SparkContext.jarOfObject(this)
core/src/main/scala/org/apache/spark/api/python/PythonUtils.scala-
pythonPath.mkString(File.pathSeparator)
```

FaultToleranceTest, which isn't actually run in our tests (since it needs a 
bunch of manual Docker setup to work):

```
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  val 
zk =  SparkCuratorUtil.newClient(conf)
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  var 
numPassed = 0
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  var 
numFailed = 0
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala:  val 
sparkHome = System.getenv(SPARK_HOME)
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  
assertTrue(sparkHome != null, Run with a valid SPARK_HOME)
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  val 
containerSparkHome = /opt/spark
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-  val 
dockerMountDir = %s:%s.format(sparkHome, containerSparkHome)
core/src/main/scala/org/apache/spark/deploy/FaultToleranceTest.scala-
```

SparkSubmitArguments, which uses this without a fallback:

```
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala-   */
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala-  
private def mergeSparkProperties(): Unit = {
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala-
// Use common defaults file, if not specified by user
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala-
if (propertiesFile == null) {
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala- 
 val sep = File.separator
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala: 
 val sparkHomeConfig = env.get(SPARK_HOME).map(sparkHome = 
s${sparkHome}${sep}conf)
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala- 
 

[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68074359
  
@JoshRosen Yes, sounds about right to me. I rebased and pushed one more 
commit to remove special `SPARK_HOME` setting in these modules too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68074482
  
  [Test build #24788 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24788/consoleFull)
 for   PR 3651 at commit 
[`2e8a0af`](https://github.com/apache/spark/commit/2e8a0afeef77df8fd9c7df406812878b22c67aa7).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68077962
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24788/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-24 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68077960
  
  [Test build #24788 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24788/consoleFull)
 for   PR 3651 at commit 
[`2e8a0af`](https://github.com/apache/spark/commit/2e8a0afeef77df8fd9c7df406812878b22c67aa7).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-23 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r22240649
  
--- Diff: extras/java8-tests/pom.xml ---
@@ -159,16 +154,6 @@
   /execution
 /executions
   /plugin
-  plugin
-groupIdorg.scalatest/groupId
-artifactIdscalatest-maven-plugin/artifactId
-executions
-  execution
-idtest/id
--- End diff --

It looks like this particular `execution` configuration is specific to 
the `java8-tests` submodule; do we need to preserve this logic?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-23 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r22240691
  
--- Diff: repl/pom.xml ---
@@ -124,6 +119,15 @@
   /environmentVariables
 /configuration
   /plugin
+  plugin
--- End diff --

Just curious, why does the REPL need special-casing for this plugin?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-23 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-68010880
  
Overall, this PR looks pretty good.  To quickly summarize the changes 
(mostly for my own benefit):

- Remove the `scalatest` and `scalatest-maven-plugin` dependencies from the 
individual subproject POMs and add a `scalatest` dependency in the root POM 
(since every subproject depends on it).
- Change every log4j.properties file to enable file appending instead of 
overwriting.

Review comments:

- We should see whether we need to preserve the special configuration of 
the ScalaTest Maven plugin in the `java8-tests` subproject.
- I like @vanzin's suggestion of adding a task that clears out the test 
logs between runs.
- I also like the idea of using a common log4j.properties file across all 
of the tests, but I'm fine with deferring that to a separate PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-23 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r22241196
  
--- Diff: extras/java8-tests/pom.xml ---
@@ -159,16 +154,6 @@
   /execution
 /executions
   /plugin
-  plugin
-groupIdorg.scalatest/groupId
-artifactIdscalatest-maven-plugin/artifactId
-executions
-  execution
-idtest/id
--- End diff --

It serves to disable `scalatest`, but `scalatest` will not run `*.java` 
tests anyway. I just gave it a spin and `surefire` runs `Java8APISuite.java` 
but `scalatest` doesn't do anything but look for tests and not find any.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-23 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r22241464
  
--- Diff: repl/pom.xml ---
@@ -124,6 +119,15 @@
   /environmentVariables
 /configuration
   /plugin
+  plugin
--- End diff --

Here I was just emulating the `scalatest` config. I'm not sure whether 
there is still a need for this or whether it's vestigial. I can try a run 
without this config to see what happens.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-12 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66772797
  
Jenkins, retest this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-12 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66773417
  
  [Test build #24408 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24408/consoleFull)
 for   PR 3651 at commit 
[`48cb3f0`](https://github.com/apache/spark/commit/48cb3f0d83451b31e37710ed1454e83c9eccb190).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-12 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66788065
  
  [Test build #24408 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24408/consoleFull)
 for   PR 3651 at commit 
[`48cb3f0`](https://github.com/apache/spark/commit/48cb3f0d83451b31e37710ed1454e83c9eccb190).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-12 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66788074
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24408/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r21697554
  
--- Diff: yarn/pom.xml ---
@@ -152,6 +147,15 @@
   /environmentVariables
 /configuration
   /plugin
+  plugin
+groupIdorg.apache.maven.plugins/groupId
+artifactIdmaven-surefire-plugin/artifactId
+configuration
+  environmentVariables
+SPARK_HOME${basedir}/../../SPARK_HOME
--- End diff --

OK, in the name of keeping it simple I might not touch this this time. 
Since this occurs 2 places only, it doesn't save much.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r21699001
  
--- Diff: pom.xml ---
@@ -941,19 +950,38 @@
 forktrue/fork
   /configuration
 /plugin
+!-- Surefire runs all Java tests --
 plugin
   groupIdorg.apache.maven.plugins/groupId
   artifactIdmaven-surefire-plugin/artifactId
-  version2.17/version
+  version2.18/version
+  !-- Note config is repeated in scalatest config --
   configuration
-!-- Uses scalatest instead --
-skipTeststrue/skipTests
+includes
+  include**/Test*.java/include
+  include**/*Test.java/include
+  include**/*TestCase.java/include
+  include**/*Suite.java/include
+/includes
+
reportsDirectory${project.build.directory}/surefire-reports/reportsDirectory
+argLine-Xmx3g -XX:MaxPermSize=${MaxPermGen} 
-XX:ReservedCodeCacheSize=512m/argLine
+systemProperties
+  java.awt.headlesstrue/java.awt.headless
+  
spark.test.home${session.executionRootDirectory}/spark.test.home
+  spark.testing1/spark.testing
+  spark.ui.enabledfalse/spark.ui.enabled
+  
spark.ui.showConsoleProgressfalse/spark.ui.showConsoleProgress
+  
spark.executor.extraClassPath${test_classpath}/spark.executor.extraClassPath
+  
spark.driver.allowMultipleContextstrue/spark.driver.allowMultipleContexts
+/systemProperties
   /configuration
 /plugin
+!-- Scalatest runs all Scala tests --
 plugin
   groupIdorg.scalatest/groupId
   artifactIdscalatest-maven-plugin/artifactId
   version1.0/version
+  !-- Note config is repeated in surefire config --
   configuration
 
reportsDirectory${project.build.directory}/surefire-reports/reportsDirectory
--- End diff --

No, the files underneath are named by test suite, so they won't collide. I 
double-checked just now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-9398
  
Good point about `log4j.appender.file.append=false`. It looks like the 
Scala tests overwrite. Hm, why not set append to `true` indeed? it's in 
`target`, so gets deleted by `clean`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66676690
  
  [Test build #24375 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24375/consoleFull)
 for   PR 3651 at commit 
[`48cb3f0`](https://github.com/apache/spark/commit/48cb3f0d83451b31e37710ed1454e83c9eccb190).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66690936
  
  [Test build #24375 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24375/consoleFull)
 for   PR 3651 at commit 
[`48cb3f0`](https://github.com/apache/spark/commit/48cb3f0d83451b31e37710ed1454e83c9eccb190).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66690943
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24375/
Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-11 Thread vanzin
Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66712690
  
The only issue with using `append = true` is that multiple test invocations 
will just keep appending to the file, potentially making debugging a little 
more confusing. Not a big deal, I think. Maybe adding a task in the root pom 
that runs before surefire/scalatest and just deletes that file? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66428269
  
I'm pretty convinced this works now. I'm diffing the test run output 
between master and this branch, and the scala tests are the same. The only 
visible differences are that `scalatest` turns up in every module, and of 
course, output from `surefire` now.

Note that I did _not_ enable assertions in SBT now, which I mentioned in a 
related conversation. There's another issue with it tracked in 
http://issues.apache.org/jira/browse/SPARK-4814

I also think this is a predecessor to 
https://issues.apache.org/jira/browse/SPARK-3431

Let's see what Jenkins says. I'm calling this no longer a WIP.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66435431
  
  [Test build #24303 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24303/consoleFull)
 for   PR 3651 at commit 
[`11bd041`](https://github.com/apache/spark/commit/11bd041909a20b6d7c1b5074d6b78133aa1ff547).
 * This patch **fails Spark unit tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66435437
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24303/
Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread srowen
Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66435761
  
Jenkins, retest this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r21641703
  
--- Diff: yarn/pom.xml ---
@@ -152,6 +147,15 @@
   /environmentVariables
 /configuration
   /plugin
+  plugin
+groupIdorg.apache.maven.plugins/groupId
+artifactIdmaven-surefire-plugin/artifactId
+configuration
+  environmentVariables
+SPARK_HOME${basedir}/../../SPARK_HOME
--- End diff --

This is ok, but a little verbose. What I've seen other projects do is 
define a pathToRoot variable that is referenced in the common configuration 
in the root pom. Then modules that rely on this env variable being set can 
override pathToRoot to have the right relative path.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread vanzin
Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/3651#discussion_r21642041
  
--- Diff: pom.xml ---
@@ -941,19 +950,38 @@
 forktrue/fork
   /configuration
 /plugin
+!-- Surefire runs all Java tests --
 plugin
   groupIdorg.apache.maven.plugins/groupId
   artifactIdmaven-surefire-plugin/artifactId
-  version2.17/version
+  version2.18/version
+  !-- Note config is repeated in scalatest config --
   configuration
-!-- Uses scalatest instead --
-skipTeststrue/skipTests
+includes
+  include**/Test*.java/include
+  include**/*Test.java/include
+  include**/*TestCase.java/include
+  include**/*Suite.java/include
+/includes
+
reportsDirectory${project.build.directory}/surefire-reports/reportsDirectory
+argLine-Xmx3g -XX:MaxPermSize=${MaxPermGen} 
-XX:ReservedCodeCacheSize=512m/argLine
+systemProperties
+  java.awt.headlesstrue/java.awt.headless
+  
spark.test.home${session.executionRootDirectory}/spark.test.home
+  spark.testing1/spark.testing
+  spark.ui.enabledfalse/spark.ui.enabled
+  
spark.ui.showConsoleProgressfalse/spark.ui.showConsoleProgress
+  
spark.executor.extraClassPath${test_classpath}/spark.executor.extraClassPath
+  
spark.driver.allowMultipleContextstrue/spark.driver.allowMultipleContexts
+/systemProperties
   /configuration
 /plugin
+!-- Scalatest runs all Scala tests --
 plugin
   groupIdorg.scalatest/groupId
   artifactIdscalatest-maven-plugin/artifactId
   version1.0/version
+  !-- Note config is repeated in surefire config --
   configuration
 
reportsDirectory${project.build.directory}/surefire-reports/reportsDirectory
--- End diff --

Will this cause the surefire reports to be overwritten? Or are the plugins 
smart enogh to append to the generated XML files instead? (Or maybe there isn't 
any summary file, just per-test files, and it's all fine?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...

2014-12-10 Thread vanzin
Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/3651#issuecomment-66533431
  
So, one problem I can think of is that the test log4j.properties has this:

log4j.appender.file.append=false

Meaning that the second test plugin will overwrite the log file generated 
by the first one. (In this case, I think scalatest will always run last, so 
you'll never see the log file for the surefire run.)

Two ways I see to fix it:
* set append to true, and clean up that file before running the tests
* parameterize the log file name so that each test run writes to a 
different file

Also, bonus point if you place the test log4j.properties in a shared 
location and nuke all the duplicate ones in each module. :-) (Although that 
might be trying to squeeze too much cleanup into this patch.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org