Repository: spark
Updated Branches:
  refs/heads/master 54cda0deb -> 1f90c5e21


[SPARK-8505] [SPARKR] Add settings to kick `lint-r` from `./dev/run-test.py`

JoshRosen we'd like to check the SparkR source code with the `dev/lint-r` 
script on the Jenkins. I tried to incorporate the script into 
`dev/run-test.py`. Could you review it when you have time?

shivaram I modified `dev/lint-r` and `dev/lint-r.R` to install lintr package 
into a local directory(`R/lib/`) and to exit with a lint status. Could you 
review it?

- [[SPARK-8505] Add settings to kick `lint-r` from `./dev/run-test.py` - ASF 
JIRA](https://issues.apache.org/jira/browse/SPARK-8505)

Author: Yu ISHIKAWA <yuu.ishik...@gmail.com>

Closes #7883 from yu-iskw/SPARK-8505.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/1f90c5e2
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/1f90c5e2
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/1f90c5e2

Branch: refs/heads/master
Commit: 1f90c5e2198bcf49e115d97ec300c17c1be4dcb4
Parents: 54cda0d
Author: Yu ISHIKAWA <yuu.ishik...@gmail.com>
Authored: Thu Aug 27 19:38:53 2015 -0700
Committer: Shivaram Venkataraman <shiva...@cs.berkeley.edu>
Committed: Thu Aug 27 19:38:53 2015 -0700

----------------------------------------------------------------------
 dev/lint-r             | 11 +++++++++++
 dev/lint-r.R           | 12 +++++++-----
 dev/run-tests-codes.sh | 13 +++++++------
 dev/run-tests-jenkins  |  2 ++
 dev/run-tests.py       | 21 ++++++++++++++++++++-
 5 files changed, 47 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/spark/blob/1f90c5e2/dev/lint-r
----------------------------------------------------------------------
diff --git a/dev/lint-r b/dev/lint-r
index 7d5f4cd..c15d57a 100755
--- a/dev/lint-r
+++ b/dev/lint-r
@@ -28,3 +28,14 @@ if ! type "Rscript" > /dev/null; then
 fi
 
 `which Rscript` --vanilla "$SPARK_ROOT_DIR/dev/lint-r.R" "$SPARK_ROOT_DIR" | 
tee "$LINT_R_REPORT_FILE_NAME"
+
+NUM_LINES=`wc -l < "$LINT_R_REPORT_FILE_NAME"`
+if [ "$NUM_LINES" = "0" ] ; then
+  lint_status=0
+  echo "lintr checks passed."
+else
+  lint_status=1
+  echo "lintr checks failed."
+fi
+
+exit "$lint_status"

http://git-wip-us.apache.org/repos/asf/spark/blob/1f90c5e2/dev/lint-r.R
----------------------------------------------------------------------
diff --git a/dev/lint-r.R b/dev/lint-r.R
index 48bd624..999eef5 100644
--- a/dev/lint-r.R
+++ b/dev/lint-r.R
@@ -17,8 +17,14 @@
 
 argv <- commandArgs(TRUE)
 SPARK_ROOT_DIR <- as.character(argv[1])
+LOCAL_LIB_LOC <- file.path(SPARK_ROOT_DIR, "R", "lib")
 
-# Installs lintr from Github.
+# Checks if SparkR is installed in a local directory.
+if (! library(SparkR, lib.loc = LOCAL_LIB_LOC, logical.return = TRUE)) {
+  stop("You should install SparkR in a local directory with 
`R/install-dev.sh`.")
+}
+
+# Installs lintr from Github in a local directory.
 # NOTE: The CRAN's version is too old to adapt to our rules.
 if ("lintr" %in% row.names(installed.packages())  == FALSE) {
   devtools::install_github("jimhester/lintr")
@@ -27,9 +33,5 @@ if ("lintr" %in% row.names(installed.packages())  == FALSE) {
 library(lintr)
 library(methods)
 library(testthat)
-if (! library(SparkR, lib.loc = file.path(SPARK_ROOT_DIR, "R", "lib"), 
logical.return = TRUE)) {
-  stop("You should install SparkR in a local directory with 
`R/install-dev.sh`.")
-}
-
 path.to.package <- file.path(SPARK_ROOT_DIR, "R", "pkg")
 lint_package(path.to.package, cache = FALSE)

http://git-wip-us.apache.org/repos/asf/spark/blob/1f90c5e2/dev/run-tests-codes.sh
----------------------------------------------------------------------
diff --git a/dev/run-tests-codes.sh b/dev/run-tests-codes.sh
index f4b238e..1f16790 100644
--- a/dev/run-tests-codes.sh
+++ b/dev/run-tests-codes.sh
@@ -21,9 +21,10 @@ readonly BLOCK_GENERAL=10
 readonly BLOCK_RAT=11
 readonly BLOCK_SCALA_STYLE=12
 readonly BLOCK_PYTHON_STYLE=13
-readonly BLOCK_DOCUMENTATION=14
-readonly BLOCK_BUILD=15
-readonly BLOCK_MIMA=16
-readonly BLOCK_SPARK_UNIT_TESTS=17
-readonly BLOCK_PYSPARK_UNIT_TESTS=18
-readonly BLOCK_SPARKR_UNIT_TESTS=19
+readonly BLOCK_R_STYLE=14
+readonly BLOCK_DOCUMENTATION=15
+readonly BLOCK_BUILD=16
+readonly BLOCK_MIMA=17
+readonly BLOCK_SPARK_UNIT_TESTS=18
+readonly BLOCK_PYSPARK_UNIT_TESTS=19
+readonly BLOCK_SPARKR_UNIT_TESTS=20

http://git-wip-us.apache.org/repos/asf/spark/blob/1f90c5e2/dev/run-tests-jenkins
----------------------------------------------------------------------
diff --git a/dev/run-tests-jenkins b/dev/run-tests-jenkins
index f144c05..39cf54f 100755
--- a/dev/run-tests-jenkins
+++ b/dev/run-tests-jenkins
@@ -210,6 +210,8 @@ done
       failing_test="Scala style tests"
     elif [ "$test_result" -eq "$BLOCK_PYTHON_STYLE" ]; then
       failing_test="Python style tests"
+    elif [ "$test_result" -eq "$BLOCK_R_STYLE" ]; then
+      failing_test="R style tests"
     elif [ "$test_result" -eq "$BLOCK_DOCUMENTATION" ]; then
       failing_test="to generate documentation"
     elif [ "$test_result" -eq "$BLOCK_BUILD" ]; then

http://git-wip-us.apache.org/repos/asf/spark/blob/1f90c5e2/dev/run-tests.py
----------------------------------------------------------------------
diff --git a/dev/run-tests.py b/dev/run-tests.py
index f689425..4fd703a 100755
--- a/dev/run-tests.py
+++ b/dev/run-tests.py
@@ -209,6 +209,18 @@ def run_python_style_checks():
     run_cmd([os.path.join(SPARK_HOME, "dev", "lint-python")])
 
 
+def run_sparkr_style_checks():
+    set_title_and_block("Running R style checks", "BLOCK_R_STYLE")
+
+    if which("R"):
+        # R style check should be executed after `install-dev.sh`.
+        # Since warnings about `no visible global function definition` appear
+        # without the installation. SEE ALSO: SPARK-9121.
+        run_cmd([os.path.join(SPARK_HOME, "dev", "lint-r")])
+    else:
+        print("Ignoring SparkR style check as R was not found in PATH")
+
+
 def build_spark_documentation():
     set_title_and_block("Building Spark Documentation", "BLOCK_DOCUMENTATION")
     os.environ["PRODUCTION"] = "1 jekyll build"
@@ -387,7 +399,6 @@ def run_sparkr_tests():
     set_title_and_block("Running SparkR tests", "BLOCK_SPARKR_UNIT_TESTS")
 
     if which("R"):
-        run_cmd([os.path.join(SPARK_HOME, "R", "install-dev.sh")])
         run_cmd([os.path.join(SPARK_HOME, "R", "run-tests.sh")])
     else:
         print("Ignoring SparkR tests as R was not found in PATH")
@@ -438,6 +449,12 @@ def main():
     if java_version.minor < 8:
         print("[warn] Java 8 tests will not run because JDK version is < 1.8.")
 
+    # install SparkR
+    if which("R"):
+        run_cmd([os.path.join(SPARK_HOME, "R", "install-dev.sh")])
+    else:
+        print("Can't install SparkR as R is was not found in PATH")
+
     if os.environ.get("AMPLAB_JENKINS"):
         # if we're on the Amplab Jenkins build servers setup variables
         # to reflect the environment settings
@@ -485,6 +502,8 @@ def main():
         run_scala_style_checks()
     if not changed_files or any(f.endswith(".py") for f in changed_files):
         run_python_style_checks()
+    if not changed_files or any(f.endswith(".R") for f in changed_files):
+        run_sparkr_style_checks()
 
     # determine if docs were changed and if we're inside the amplab environment
     # note - the below commented out until *all* Jenkins workers can get 
`jekyll` installed


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

Reply via email to