[GitHub] spark pull request: SPARK-4159 [CORE] Maven build doesn't run JUni...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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