Changed hostname in integration tests to deal with weirdness in resol…

…ving localhost on Travis
uptake · Jun 2, 2018 · 6081b87 · 6081b87
1 parent 9fe9a22
commit 6081b87
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 23 deletions.
diff --git a/R/elasticsearch_eda_funs.R b/R/elasticsearch_eda_funs.R
@@ -283,8 +283,14 @@ get_fields <- function(es_host
 #' @importFrom data.table data.table
 #' @importFrom utils read.table
 .process_alias <- function(alias_string) {
+
     # process the string provided by the /_cat/aliases API into a data.frame and then a data.table
-    aliasDT <- data.table::data.table(utils::read.table(text = alias_string, stringsAsFactors = FALSE))
+    aliasDT <- data.table::data.table(
+      utils::read.table(
+        text = alias_string
+        , stringsAsFactors = FALSE
+      )
+    )
 
     # return only the first two columns
     return(aliasDT[, .(alias = V1, index = V2)])

diff --git a/R/es_search.R b/R/es_search.R
@@ -690,6 +690,7 @@ es_search <- function(es_host
         , url = reqURL
         , body = query_body
     )
+    print(httr::content(result, as = "text"))
     httr::stop_for_status(result)
     result <- httr::content(result, as = "text")
 

diff --git a/R/get_fields.R b/R/get_fields.R
@@ -120,7 +120,8 @@ get_fields <- function(es_host
     httr::stop_for_status(result)
     resultContent <- httr::content(result, as = 'text')
 
-    if (is.null(resultContent)) {
+    # NOTE: with ES6, this results in an empty string instead of a NULL
+    if (is.null(resultContent) || identical(resultContent, "")) {
         # there are no aliases in this Elasticsearch cluster
         return(invisible(NULL))
     } else {
@@ -133,6 +134,7 @@ get_fields <- function(es_host
 #' @importFrom data.table data.table
 #' @importFrom utils read.table
 .process_alias <- function(alias_string) {
+
     # process the string provided by the /_cat/aliases API into a data.frame and then a data.table
     aliasDT <- data.table::data.table(
         utils::read.table(

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -1,9 +1,9 @@
 version: '3'
 services:
   es_6:
-    image: "docker.elastic.co/elasticsearch/elasticsearch:6.2.0"
+    image: "docker.elastic.co/elasticsearch/elasticsearch:5.5.0"
     environment:
-      - "discovery.type=single-node"
+      #- "discovery.type=single-node"
       - "xpack.security.enabled=false"
     ports:
       - "9200:9200"
diff --git a/setup_es.sh b/setup_es.sh
@@ -37,4 +37,4 @@ curl --silent \
 curl -X POST "http://${ES_HOST}:9200/_refresh"
 
 # Check that we got something
-curl -X GET "http://${ES_HOST}/shakespeare/_search?size=1"
+curl -X GET "http://${ES_HOST}:9200/shakespeare/_search?size=1"
diff --git a/tests/testthat/test-es_search.R b/tests/testthat/test-es_search.R
@@ -20,6 +20,32 @@ test_that("es_search should reject NULL index", {
     }, regexp = "es_index is not a string")
 })
 
+# Should reject bad queries
+test_that("es_search should reject malformed queries", {
+    # Length greater than 1
+    expect_error({
+        es_search(
+            es_host = "http://mycompany.com:9200"
+            , es_index = "_all"
+            , query = c(
+                    '{"_source": {"include": ["stuff.*"]},'
+                    , '{"aggs": {"superman": {"terms": {"field": "hi"}}}}}'
+                )
+        )
+    }, regexp = "You gave an object of length 2")
+
+    # Specified as a list (like you might get from jsonlite::fromJSON)
+    expect_error({
+        es_search(
+            es_host = "http://mycompany.com:9200"
+            , es_index = "_all"
+            , query = list(
+                '{"_source": {"include": ["stuff.*"]},{"aggs": {"superman": {"terms": {"field": "hi"}}}}}'
+            )
+        )
+    }, regexp = "query_body should be a single string")
+})
+
 #---- .ConvertToSec
 
 # .ConvertToSec should work for seconds

diff --git a/tests/testthat/test-integration.R b/tests/testthat/test-integration.R
@@ -23,10 +23,10 @@ futile.logger::flog.threshold(0)
         testthat::skip_on_cran()
 
         outDT <- es_search(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_index = "shakespeare"
-            , max_hits = 48
-            , size = 48
+            , max_hits = 100
+            , size = 100
         )
 
        expect_true(data.table::is.data.table(outDT))
@@ -36,35 +36,35 @@ futile.logger::flog.threshold(0)
         testthat::skip_on_cran()
 
         outDT <- es_search(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_index = "shakespeare"
-            , max_hits = 50
+            , max_hits = 30
             , size = 2
         )
         expect_true(data.table::is.data.table(outDT))
-        expect_true(nrow(outDT) == 48)
+        expect_true(nrow(outDT) == 30)
     })
 
     test_that("es_search works in single-threaded mode", {
         testthat::skip_on_cran()
 
         outDT <- es_search(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_index = "shakespeare"
-            , max_hits = 50
+            , max_hits = 30
             , size = 2
             , n_cores = 1
         )
         expect_true(data.table::is.data.table(outDT))
-        expect_true(nrow(outDT) == 48)
+        expect_true(nrow(outDT) == 30)
     })
 
     test_that("es_search rejects scrolls longer than 1 hour", {
         testthat::skip_on_cran()
 
         expect_error({
             outDT <- es_search(
-                es_host = "http://localhost:9200"
+                es_host = "http://127.0.0.1:9200"
                 , es_index = "shakespeare"
                 , max_hits = 100
                 , size = 100
@@ -79,7 +79,7 @@ futile.logger::flog.threshold(0)
 
         expect_warning({
             outDT <- es_search(
-                es_host = "http://localhost:9200"
+                es_host = "http://127.0.0.1:9200"
                 , es_index = "shakespeare"
                 , max_hits = 9999
             )
@@ -93,7 +93,7 @@ futile.logger::flog.threshold(0)
 
         expect_warning({
             outDT <- es_search(
-                es_host = "http://localhost:9200"
+                es_host = "http://127.0.0.1:9200"
                 , es_index = "shakespeare"
                 , max_hits = 12
                 , size = 7
@@ -110,7 +110,7 @@ futile.logger::flog.threshold(0)
         #       and I want to avoid exposing our tests to changes in the query DSL
         expect_warning({
             outDT <- es_search(
-                es_host = "http://localhost:9200"
+                es_host = "http://127.0.0.1:9200"
                 , es_index = "empty_index"
             )
         }, regexp = "Query is syntactically valid but 0 documents were matched")
@@ -122,14 +122,14 @@ futile.logger::flog.threshold(0)
         testthat::skip_on_cran()
 
         outDT <- es_search(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_index = "shakespeare"
             , max_hits = 100
             , query = '{"aggs": {"thing": {"terms": {"field": "speaker", "size": 12}}}}'
         )
 
         expect_true(data.table::is.data.table(outDT))
-        expect_true(nrow(outDT) == 3)
+        expect_true(nrow(outDT) == 12)
         expect_named(
             outDT
             , c("thing", "doc_count")
@@ -145,7 +145,7 @@ futile.logger::flog.threshold(0)
         testthat::skip_on_cran()
 
         outDT <- es_search(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_index = "shakespeare"
             , max_hits = 100
             , query = '{"aggs": {"name_i_picked": {"terms": {"field": "speaker", "size": 12}}}}'
@@ -161,7 +161,7 @@ futile.logger::flog.threshold(0)
 
         # ther stuff we might as well test
         expect_true(data.table::is.data.table(outDT))
-        expect_true(nrow(outDT) == 3)
+        expect_true(nrow(outDT) == 12)
         expect_true(is.numeric(outDT[, doc_count]))
         expect_true(is.character(outDT[, name_i_picked]))
         expect_true(all(outDT[, doc_count > 0]))
@@ -173,7 +173,7 @@ futile.logger::flog.threshold(0)
         testthat::skip_on_cran()
 
         fieldDT <- get_fields(
-            es_host = "http://localhost:9200"
+            es_host = "http://127.0.0.1:9200"
             , es_indices = "_all"
         )
         expect_true(data.table::is.data.table(fieldDT))