[SPARK-6765] Fix test code style for SQL #5412

rxin · 2015-04-08T06:58:20Z

So we can turn style checker on for test code.

SparkQA · 2015-04-08T07:03:24Z

Test build #29842 has started for PR 5412 at commit ef4ec48.

SparkQA · 2015-04-08T07:19:43Z

Test build #29842 has finished for PR 5412 at commit ef4ec48.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T07:19:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29842/
Test FAILed.

liancheng · 2015-04-08T07:22:36Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala

@@ -202,7 +204,7 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {

  case class UnresolvedTestPlan() extends LeafNode {
    override lazy val resolved = false


Do we also want public fields have explicit type too?

Yes - but there is no rule for it yet.

liancheng · 2015-04-08T07:26:29Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala

      fail(
        s"""
          |== FAIL: Plans do not match ===
          |${sideBySide(normalized1.treeString, normalized2.treeString).mkString("\n")}
        """.stripMargin)
+      }


Indentation is off here.

SparkQA · 2015-04-08T07:28:29Z

Test build #29846 has started for PR 5412 at commit f2f4348.

SparkQA · 2015-04-08T08:17:35Z

Test build #29846 has finished for PR 5412 at commit f2f4348.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T08:17:38Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29846/
Test FAILed.

liancheng · 2015-04-08T08:59:14Z

sql/core/src/test/scala/org/apache/spark/sql/QueryTest.scala

@@ -67,7 +67,7 @@ class QueryTest extends PlanTest {
    checkAnswer(df, Seq(expectedAnswer))
  }

-  def sqlTest(sqlString: String, expectedAnswer: Seq[Row])(implicit sqlContext: SQLContext): Unit = {
+  def sqlTest(sqlString: String, expectedAnswer: Seq[Row])(implicit sqlContext: SQLContext) {


Why removing return type of this one?

it was too long - either wrap it, or just remove :Unit

I believe this form of method is going to be deprecated.

SparkQA · 2015-04-08T20:38:37Z

Test build #29887 has started for PR 5412 at commit 0b03fbb.

SparkQA · 2015-04-08T20:43:08Z

Test build #29888 has started for PR 5412 at commit 82d0944.

SparkQA · 2015-04-08T20:57:26Z

Test build #29887 has finished for PR 5412 at commit 0b03fbb.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T20:57:29Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29887/
Test FAILed.

SparkQA · 2015-04-08T21:00:03Z

Test build #29888 has finished for PR 5412 at commit 82d0944.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T21:00:06Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29888/
Test FAILed.

SparkQA · 2015-04-08T21:03:32Z

Test build #29890 has started for PR 5412 at commit 8c7250a.

SparkQA · 2015-04-08T21:25:14Z

Test build #29890 has finished for PR 5412 at commit 8c7250a.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-08T21:25:17Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29890/
Test FAILed.

SparkQA · 2015-04-09T00:28:22Z

Test build #29904 has started for PR 5412 at commit 9098a31.

SparkQA · 2015-04-09T01:52:21Z

Test build #29904 has finished for PR 5412 at commit 9098a31.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-09T01:52:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29904/
Test PASSed.

marmbrus · 2015-04-09T02:36:23Z

For @rxin 's sanity I'll merge this now :)

We can update the final minor comments in followups.

marmbrus · 2015-04-09T02:40:10Z

Actually, apache git is down :(

rxin · 2015-04-09T03:35:41Z

Ok I've merged it.

Turn scalastyle on for all test code. Most of the violations have been resolved in my previous pull requests: Core: #5484 SQL: #5412 MLlib: #5411 GraphX: #5410 Streaming: #5409 Author: Reynold Xin <rxin@databricks.com> Closes #5486 from rxin/test-style-enable and squashes the following commits: 01683de [Reynold Xin] Fixed new code. a4ab46e [Reynold Xin] Fixed tests. 20adbc8 [Reynold Xin] Missed one violation. 5e36521 [Reynold Xin] [SPARK-6765] Enable scalastyle on test code.

rxin added 3 commits April 7, 2015 23:31

catalyst module

04ec7ac

sql module

7e0db5e

Hive module.

ef4ec48

liancheng reviewed Apr 8, 2015
View reviewed changes

oops.

f2f4348

liancheng reviewed Apr 8, 2015
View reviewed changes

code review.

0b03fbb

Indentation.

82d0944

Fix compilation.

8c7250a

One more compilation error ...

9098a31

asfgit closed this in 1b2aab8 Apr 9, 2015

rxin mentioned this pull request Apr 13, 2015

[SPARK-6765] Enable scalastyle on test code. #5486

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-6765] Fix test code style for SQL #5412

[SPARK-6765] Fix test code style for SQL #5412

rxin commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

liancheng Apr 8, 2015

rxin Apr 8, 2015

liancheng Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

liancheng Apr 8, 2015

rxin Apr 8, 2015

marmbrus Apr 9, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 9, 2015

SparkQA commented Apr 9, 2015

AmplabJenkins commented Apr 9, 2015

marmbrus commented Apr 9, 2015

marmbrus commented Apr 9, 2015

rxin commented Apr 9, 2015

		@@ -202,7 +204,7 @@ class AnalysisSuite extends FunSuite with BeforeAndAfter {

		case class UnresolvedTestPlan() extends LeafNode {
		override lazy val resolved = false

[SPARK-6765] Fix test code style for SQL #5412

[SPARK-6765] Fix test code style for SQL #5412

Conversation

rxin commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

liancheng Apr 8, 2015

Choose a reason for hiding this comment

rxin Apr 8, 2015

Choose a reason for hiding this comment

liancheng Apr 8, 2015

Choose a reason for hiding this comment

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

liancheng Apr 8, 2015

Choose a reason for hiding this comment

rxin Apr 8, 2015

Choose a reason for hiding this comment

marmbrus Apr 9, 2015

Choose a reason for hiding this comment

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 8, 2015

SparkQA commented Apr 8, 2015

AmplabJenkins commented Apr 8, 2015

SparkQA commented Apr 9, 2015

SparkQA commented Apr 9, 2015

AmplabJenkins commented Apr 9, 2015

marmbrus commented Apr 9, 2015

marmbrus commented Apr 9, 2015

rxin commented Apr 9, 2015