[SPARK-22269][BUILD] Run Java linter via SBT for Jenkins #21399

HyukjinKwon · 2018-05-22T15:35:08Z

What changes were proposed in this pull request?

This PR proposes to check Java lint via SBT for Jenkins. It uses the SBT wrapper for checkstyle.

I manually tested. If we build the codes once, running this script takes 2 mins at maximum in my local:

Test codes:

Checkstyle failed at following occurrences:
[error] Checkstyle error found in /.../spark/core/src/test/java/test/org/apache/spark/JavaAPISuite.java:82: Line is longer than 100 characters (found 103).
[error] 1 issue(s) found in Checkstyle report: /.../spark/core/target/checkstyle-test-report.xml
[error] Checkstyle error found in /.../spark/sql/hive/src/test/java/org/apache/spark/sql/hive/JavaDataFrameSuite.java:84: Line is longer than 100 characters (found 115).
[error] 1 issue(s) found in Checkstyle report: /.../spark/sql/hive/target/checkstyle-test-report.xml
...

Main codes:

Checkstyle failed at following occurrences:
[error] Checkstyle error found in /.../spark/sql/core/src/main/java/org/apache/spark/sql/sources/v2/reader/InputPartition.java:39: Line is longer than 100 characters (found 104).
[error] Checkstyle error found in /.../spark/sql/core/src/main/java/org/apache/spark/sql/sources/v2/reader/InputPartitionReader.java:26: Line is longer than 100 characters (found 110).
[error] Checkstyle error found in /.../spark/sql/core/src/main/java/org/apache/spark/sql/sources/v2/reader/InputPartitionReader.java:30: Line is longer than 100 characters (found 104).
...

How was this patch tested?

Manually tested. Jenkins build should test this.

SparkQA · 2018-05-22T15:38:04Z

Test build #90973 has started for PR 21399 at commit 15e4cf8.

dongjoon-hyun · 2018-05-22T15:46:12Z

This is the one? Finally, nice! @HyukjinKwon .

This reverts commit b004047.

HyukjinKwon · 2018-05-22T15:52:18Z

Thanks @dongjoon-hyun. BTW, I am pushing some commits to check if it works in Jenkins and show the results.

SparkQA · 2018-05-22T16:03:16Z

Test build #90976 has finished for PR 21399 at commit b004047.

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

advancedxy · 2018-05-22T16:03:52Z

project/SparkBuild.scala

  )
 }

+object CheckStyle {


How about call it as CheckJavaStyle.

CheckStyle seems a broad concept which should include scala style chceck.

And dev/sbt-checkstyle -> dev/sbt-checkjavastyle or similar

sbt-checkstyle is actually a package name which is consistent in dev scripts.

CheckStyle is also a package name like Unidoc.

Let me fix CheckStyle to Checkstyle while I am here btw.

Ah, I see.

Thanks for clarification as it's a bit confusing.

vanzin · 2018-05-22T16:29:20Z

project/plugins.sbt

+libraryDependencies += "com.puppycrawl.tools" % "checkstyle" % "8.2"
+
+// checkstyle uses guava 23.0.
+libraryDependencies += "com.google.guava" % "guava" % "23.0"


I'm a little surprised this is not causing other problems... when I was testing this, it was overriding Spark's own Guava import (which is version 14).

But if it works, awesome.

Yea but it's plugin dependency here. I was a little surprised too because I was stuck here for a while.

HyukjinKwon · 2018-05-22T16:43:45Z

Let me push some more commits in main code and test code too to double check.

This reverts commit e480ecc.

This reverts commit 1df7285.

SparkQA · 2018-05-22T17:00:21Z

Test build #90983 has finished for PR 21399 at commit 6e4f6c3.

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

SparkQA · 2018-05-22T17:04:00Z

Test build #90982 has finished for PR 21399 at commit 0cf6de0.

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

SparkQA · 2018-05-22T19:42:20Z

Test build #90980 has finished for PR 21399 at commit 54d070d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-05-22T19:54:29Z

Test build #90977 has finished for PR 21399 at commit 558b1be.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-05-22T20:57:47Z

Test build #90984 has finished for PR 21399 at commit 7bb0eb3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2018-05-22T22:49:02Z

retest this please

SparkQA · 2018-05-23T03:05:35Z

Test build #91000 has finished for PR 21399 at commit 7bb0eb3.

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

HyukjinKwon · 2018-05-23T03:24:39Z

@vanzin, I will leave it to you as I believe you are the most appropriate one to review this - I assume the approach you initially took is the same with this. I feel quite sure on the current change FWIW.

vanzin

Let me check what things look like after the build is finished, but otherwise this looks good.

vanzin · 2018-05-23T20:12:48Z

dev/run-tests.py

                                for f in changed_files):
-        # run_java_style_checks()
-        pass
+        run_java_style_checks()


This is really minor, but now you'll be running this when building with maven too. That could be avoided by checking that the build is not using maven; and also changing the maven target from package to verify so that these checks run.

You can add [build-maven] to the PR title to try it out (or locally with AMPLAB_JENKINS_BUILD_TOOL=maven).

no problem.

vanzin · 2018-05-23T20:23:16Z

Checked locally and looks good. Feel free to merge if you don't want to address the comment above in this PR.

HyukjinKwon · 2018-05-24T00:36:54Z

retest this please

SparkQA · 2018-05-24T04:21:59Z

Test build #91076 has finished for PR 21399 at commit 7bb0eb3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

This reverts commit e480ecc.

This reverts commit 294e189.

SparkQA · 2018-05-24T05:41:06Z

Test build #91084 has finished for PR 21399 at commit 294e189.

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

HyukjinKwon · 2018-05-24T06:17:08Z

^ this one was ran with Maven. I am merging this in.

HyukjinKwon · 2018-05-24T06:20:20Z

Merged to master.

Thanks all, @dongjoon-hyun, @advancedxy, @vanzin and @felixcheung.

SparkQA · 2018-05-24T07:05:02Z

Test build #91085 has finished for PR 21399 at commit 6943ff8.

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

## What changes were proposed in this pull request? #12980 added Travis CI file mainly for linter because we disabled Java lint check in Jenkins. It's enabled as of #21399 and now SBT runs it. Looks we can now remove the file added before. ## How was this patch tested? N/A Closes #22665 Closes #22667 from HyukjinKwon/SPARK-25673. Authored-by: hyukjinkwon <[email protected]> Signed-off-by: hyukjinkwon <[email protected]> (cherry picked from commit 2199224) Signed-off-by: hyukjinkwon <[email protected]>

## What changes were proposed in this pull request? apache#12980 added Travis CI file mainly for linter because we disabled Java lint check in Jenkins. It's enabled as of apache#21399 and now SBT runs it. Looks we can now remove the file added before. ## How was this patch tested? N/A Closes apache#22665 Closes apache#22667 from HyukjinKwon/SPARK-25673. Authored-by: hyukjinkwon <[email protected]> Signed-off-by: hyukjinkwon <[email protected]>

Run Java linter with Jenkins via SBT

15e4cf8

Weird Java file to check the Jenkins output

b004047

HyukjinKwon added 2 commits May 22, 2018 23:50

Narrower scope (and test the weird Java file too)

81ed6da

Revert "Weird Java file to check the Jenkins output"

558b1be

This reverts commit b004047.

advancedxy reviewed May 22, 2018

View reviewed changes

CheckStyle to Checkstyle

54d070d

vanzin reviewed May 22, 2018

View reviewed changes

Weird java file in test codes

0cf6de0

HyukjinKwon added 3 commits May 23, 2018 00:46

Revert "[SPARK-24323][SQL] Fix lint-java errors"

1df7285

This reverts commit e480ecc.

Revert test codes change

6e4f6c3

Revert "Revert "[SPARK-24323][SQL] Fix lint-java errors""

7bb0eb3

This reverts commit 1df7285.

felixcheung approved these changes May 23, 2018

View reviewed changes

vanzin reviewed May 23, 2018

View reviewed changes

HyukjinKwon changed the title ~~[SPARK-22269][BUILD] Run Java linter via SBT for Jenkins~~ [SPARK-22269][BUILD][test-maven] Run Java linter via SBT for Jenkins May 24, 2018

HyukjinKwon added 2 commits May 24, 2018 13:29

Revert "[SPARK-24323][SQL] Fix lint-java errors"

294e189

This reverts commit e480ecc.

Revert "Revert "[SPARK-24323][SQL] Fix lint-java errors""

6943ff8

This reverts commit 294e189.

HyukjinKwon changed the title ~~[SPARK-22269][BUILD][test-maven] Run Java linter via SBT for Jenkins~~ [SPARK-22269][BUILD] Run Java linter via SBT for Jenkins May 24, 2018

asfgit closed this in 4a14dc0 May 24, 2018

HyukjinKwon mentioned this pull request Oct 8, 2018

[SPARK-25673][BUILD] Remove Travis CI which enables Java lint check #22667

Closed

HyukjinKwon deleted the SPARK-22269 branch October 16, 2018 12:43

HyukjinKwon mentioned this pull request Dec 10, 2019

[WIP][SPARK-30194][BUILD][test-maven] Re-enable checkstyle for Java #26820

Closed

LuciferYang mentioned this pull request Jan 17, 2025

[SPARK-50891][BUILD] Remove the explicit dependency on Guava from plugins.sbt #49550

Closed

[SPARK-22269][BUILD] Run Java linter via SBT for Jenkins #21399

[SPARK-22269][BUILD] Run Java linter via SBT for Jenkins #21399

Uh oh!

Conversation

HyukjinKwon commented May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

dongjoon-hyun commented May 22, 2018

Uh oh!

HyukjinKwon commented May 22, 2018

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

advancedxy May 22, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon May 22, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon May 22, 2018

Choose a reason for hiding this comment

Uh oh!

advancedxy May 22, 2018

Choose a reason for hiding this comment

Uh oh!

vanzin May 22, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented May 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

SparkQA commented May 22, 2018

Uh oh!

HyukjinKwon commented May 22, 2018

Uh oh!

SparkQA commented May 23, 2018

Uh oh!

HyukjinKwon commented May 23, 2018

Uh oh!

vanzin left a comment

Choose a reason for hiding this comment

Uh oh!

vanzin May 23, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon May 24, 2018

Choose a reason for hiding this comment

Uh oh!

vanzin commented May 23, 2018

Uh oh!

HyukjinKwon commented May 24, 2018

Uh oh!

SparkQA commented May 24, 2018

Uh oh!

SparkQA commented May 24, 2018

Uh oh!

HyukjinKwon commented May 24, 2018

Uh oh!

HyukjinKwon commented May 24, 2018

Uh oh!

SparkQA commented May 24, 2018

Uh oh!

Reviewers

Assignees

HyukjinKwon commented May 22, 2018 •

edited

Loading

HyukjinKwon May 22, 2018 •

edited

Loading

HyukjinKwon commented May 22, 2018 •

edited

Loading