Handle NA too with few test cases

apache · Dec 27, 2017 · 6366cf9 · 6366cf9
1 parent 3c49523
commit 6366cf9
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 7 deletions.
diff --git a/R/pkg/R/sparkR.R b/R/pkg/R/sparkR.R
@@ -580,8 +580,8 @@ setJobDescription <- function(value) {
 #'}
 #' @note setLocalProperty since 2.3.0
 setLocalProperty <- function(key, value) {
-  if (is.null(key)) {
-    stop("key should not be NULL.")
+  if (is.null(key) || is.na(key)) {
+    stop("key should not be NULL or NA.")
   }
   if (!is.null(value)) {
     value <- as.character(value)
@@ -602,11 +602,11 @@ setLocalProperty <- function(key, value) {
 #'}
 #' @note getLocalProperty since 2.3.0
 getLocalProperty <- function(key) {
-  if (is.null(key)) {
-    stop("key should not be NULL.")
+  if (is.null(key) || is.na(key)) {
+    stop("key should not be NULL or NA.")
   }
   sc <- getSparkContext()
-  invisible(callJMethod(sc, "getLocalProperty", as.character(key)))
+  callJMethod(sc, "getLocalProperty", as.character(key))
 }
 
 sparkConfToSubmitOps <- new.env()

diff --git a/R/pkg/tests/fulltests/test_context.R b/R/pkg/tests/fulltests/test_context.R
@@ -115,19 +115,27 @@ test_that("job description and local properties can be set and got", {
   expect_equal(getLocalProperty("spark.job.description"), "1234")
   setJobDescription(NULL)
   expect_equal(getLocalProperty("spark.job.description"), NULL)
+  setJobDescription(NA)
+  expect_equal(getLocalProperty("spark.job.description"), NULL)
 
   setLocalProperty("spark.scheduler.pool", "poolA")
   expect_equal(getLocalProperty("spark.scheduler.pool"), "poolA")
   setLocalProperty("spark.scheduler.pool", NULL)
   expect_equal(getLocalProperty("spark.scheduler.pool"), NULL)
+  setLocalProperty("spark.scheduler.pool", NA)
+  expect_equal(getLocalProperty("spark.scheduler.pool"), NULL)
 
   setLocalProperty(4321, 1234)
   expect_equal(getLocalProperty(4321), "1234")
   setLocalProperty(4321, NULL)
   expect_equal(getLocalProperty(4321), NULL)
+  setLocalProperty(4321, NA)
+  expect_equal(getLocalProperty(4321), NULL)
 
-  expect_error(setLocalProperty(NULL, "should fail"), "key should not be NULL.")
-  expect_error(getLocalProperty(NULL), "key should not be NULL.")
+  expect_error(setLocalProperty(NULL, "should fail"), "key should not be NULL or NA")
+  expect_error(getLocalProperty(NULL), "key should not be NULL or NA")
+  expect_error(setLocalProperty(NA, "should fail"), "key should not be NULL or NA")
+  expect_error(getLocalProperty(NA), "key should not be NULL or NA")
   sparkR.session.stop()
 })