Clean up the tests

Clean up the code in the tests by removing redundant bits and using more specific test helpers where possible.

Also, use `skip_if_offline(host = "www.opendata.nhs.scot")` in many tests to avoid failures outside of the package's control.

tests/testthat/test-check_dataset_name.R

@@ -1,20 +1,20 @@
 test_that("errors for incorrect case", {
   expect_error(
-    phsopendata:::check_dataset_name("hospital codes"),
+    check_dataset_name("hospital codes"),
     regexp = "dataset_name must be in dash-case"
   )
 })
 
 test_that("errors for incorrect type", {
   expect_error(
-    phsopendata:::check_dataset_name(20),
+    check_dataset_name(20),
     regexp = "dataset_name must be of type character"
   )
 })
 
 test_that("returns nothing for valid type and format", {
   expect_equal(
-    phsopendata:::check_dataset_name("hospital-codes"),
+    check_dataset_name("hospital-codes"),
     NULL
   )
 })

tests/testthat/test-check_res_id.R

@@ -1,27 +1,27 @@
 test_that("errors for length > 1", {
   expect_error(
-    phsopendata:::check_res_id(letters),
+    check_res_id(letters),
     regexp = "You supplied a res_id with a length of 26"
   )
 })
 
 test_that("errors for incorrect type", {
   expect_error(
-    phsopendata:::check_res_id(20),
+    check_res_id(20),
     regexp = "(must be of type character)"
   )
 })
 
 test_that("errors for invalid format", {
   expect_error(
-    phsopendata:::check_res_id("wrong format"),
+    check_res_id("wrong format"),
     regexp = "is in an invalid format."
   )
 })
 
 test_that("returns nothing for correct format/length/type", {
   expect_equal(
-    phsopendata:::check_res_id("a965ee86-0974-4c93-bbea-e839e27d7085"),
+    check_res_id("a965ee86-0974-4c93-bbea-e839e27d7085"),
     NULL
   )
 })

tests/testthat/test-dump_download.R

@@ -1,13 +1,17 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("throws error for non-existent res_ids", {
   expect_error(
-    data <- phsopendata:::dump_download("not-real"),
+    dump_download("not-real"),
     regexp = "Can't find resource with ID"
   )
 })
 
 test_that("downloads full resource", {
-  data <- phsopendata:::dump_download("a794d603-95ab-4309-8c92-b48970478c14")
+  data <- dump_download("a794d603-95ab-4309-8c92-b48970478c14")
 
   expect_equal(nrow(data), 926)
-  expect_equal(ncol(data), 15)
+  expect_length(data, 15)
+  expect_named(data)
+  expect_s3_class(data, "tbl_df")
 })

tests/testthat/test-error_check.R

@@ -1,22 +1,14 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("returns nothing if no error", {
-  content <- httr::content(
-    httr::GET(
-      phsopendata:::request_url("package_list", "")
+  expect_null(
+    error_check(phs_GET("package_list", ""))
     )
-  )
-  expect_equal(
-    phsopendata:::error_check(content), NULL
-  )
 })
 
 test_that("throws error if error in httr content", {
-  content <- httr::content(
-    httr::GET(
-      phsopendata:::request_url("datastore_search", "id=doop")
-    )
-  )
   expect_error(
-    phsopendata:::error_check(content),
+    error_check(phs_GET("datastore_search", "id=doop")),
     regexp = 'Resource "doop" was not found.'
   )
 })

tests/testthat/test-get_dataset.R

@@ -1,3 +1,5 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("returns data in the expected format", {
   data <- get_dataset(
     dataset_name = "gp-practice-populations",

tests/testthat/test-get_resource.R

@@ -1,3 +1,5 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("returns data in the expected format", {
   gp_list_apr_2021 <- "a794d603-95ab-4309-8c92-b48970478c14"
 

tests/testthat/test-get_resource_sql.R

@@ -1,3 +1,5 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("throws errors on invalid sql argument", {
   # wrong class
   expect_error(

tests/testthat/test-parse_col_select.R

@@ -1,13 +1,10 @@
 test_that("creates commas separated string from vector", {
   expect_equal(
-    phsopendata:::parse_col_select(letters[1:5]),
+    parse_col_select(letters[1:5]),
     "a,b,c,d,e"
   )
 })
 
 test_that("returns NULL if input is NULL", {
-  expect_equal(
-    phsopendata:::parse_col_select(NULL),
-    NULL
-  )
+  expect_null(parse_col_select(NULL))
 })

tests/testthat/test-parse_error.R

@@ -1,21 +1,23 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("correctly extracts error from API response", {
   content <- httr::content(
     httr::GET(
-      phsopendata:::request_url("datastore_search", "id=doop")
+      request_url("datastore_search", "id=doop")
     )
   )
   expect_equal(
-    phsopendata:::parse_error(content$error),
+    parse_error(content$error),
     "Not Found Error: Not found: Resource \"doop\" was not found."
   )
 
   content <- httr::content(
     httr::GET(
-      phsopendata:::request_url("datastore_search", "")
+      request_url("datastore_search", "")
     )
   )
   expect_equal(
-    phsopendata:::parse_error(content$error),
+    parse_error(content$error),
     "resource_id: Missing value"
   )
 })

tests/testthat/test-parse_row_filters.R

@@ -1,27 +1,27 @@
 test_that("returns NULL if `row_filters` = NULL", {
   expect_null(
-    phsopendata:::parse_row_filters(NULL)
+    parse_row_filters(NULL)
   )
 })
 
 test_that("throws error for length > 1", {
   expect_error(
-    phsopendata:::parse_row_filters(list(x = letters)),
+    parse_row_filters(list(x = letters)),
     regexp = "(list must only contain vectors of length 1.)"
   )
 })
 
 test_that("throws error for non-unique names", {
   expect_error(
-    phsopendata:::parse_row_filters(list(x = 1, x = 2)),
+    parse_row_filters(list(x = 1, x = 2)),
     regexp = "Only one filter per field is currently supported by `get_resource`"
   )
 })
 
 test_that("returns JSON string from list", {
   expect_true(
     jsonlite::validate(
-      phsopendata:::parse_row_filters(list(x = 5, y = 6))
+      parse_row_filters(list(x = 5, y = 6))
     )
   )
 })

tests/testthat/test-phs_GET.R

@@ -1,28 +1,34 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("returns httr::content", {
-  x <- phsopendata:::phs_GET("package_list", "")
-  expect_true(
-    !is.null(x$help) && !is.null(x$success)
+  content <- phs_GET("package_list", "")
+
+  expect_true(content$success)
+
+  expect_equal(
+    content$help,
+    "https://www.opendata.nhs.scot/api/3/action/help_show?name=package_list"
   )
 })
 
 test_that("error_check() works as expected", {
   # no error for valid endpoint
   expect_type(
-    phsopendata:::phs_GET("package_list", ""),
+    phs_GET("package_list", ""),
     "list"
   )
 
   # not found error
   expect_error(
-    phsopendata:::phs_GET("datastore_search", "id=doop"),
+    phs_GET("datastore_search", "id=doop"),
     regexp = 'Resource "doop" was not found.'
   )
 })
 
 test_that("request_url() works as expected", {
   # invalid action argument
   expect_error(
-    phsopendata:::phs_GET("", ""),
+    phs_GET("", ""),
     regexp = "API call failed"
   )
 })

tests/testthat/test-request_url.R

@@ -1,18 +1,18 @@
 test_that("returns correct URL format", {
   expect_equal(
-    phsopendata:::request_url("datastore_search", "id=doop"),
+    request_url("datastore_search", "id=doop"),
     "https://www.opendata.nhs.scot/api/3/action/datastore_search?id=doop"
   )
 
   expect_equal(
-    phsopendata:::request_url("dump", "id=doop"),
+    request_url("dump", "id=doop"),
     "https://www.opendata.nhs.scot/datastore/dump/id=doop?bom=true"
   )
 })
 
 test_that("rejects invalid actions", {
   expect_error(
-    phsopendata:::request_url("beep", ""),
+    request_url("beep", ""),
     regexp = "API call failed."
   )
 })

tests/testthat/test-suggest_dataset_names.R

@@ -1,6 +1,8 @@
+skip_if_offline(host = "www.opendata.nhs.scot")
+
 test_that("throws error and doesn't suggest for large distance", {
   expect_error(
-    phsopendata:::suggest_dataset_name(
+    suggest_dataset_name(
       "135987645892erhusidhnjsfdhf92ry9823hr2iuh2eiyrhwfue"
     ),
     regexp = "Can't find the dataset name"
@@ -9,7 +11,7 @@ test_that("throws error and doesn't suggest for large distance", {
 
 test_that("throws error and does suggest for close matches", {
   expect_error(
-    phsopendata:::suggest_dataset_name(
+    suggest_dataset_name(
       "rospital-codes"
     ),
     regexp = "Did you mean 'hospital-codes'?"

tests/testthat/test-use_dump_check.R

@@ -1,43 +1,40 @@
 test_that("returns true as expected", {
   # all are null
   expect_true(
-    phsopendata:::use_dump_check(list(), NULL)
+    use_dump_check(list(), NULL)
   )
 
   # rows > 99999 and all query is NULL
   expect_true(
-    suppressWarnings(
-      phsopendata:::use_dump_check(list(), 100000)
-    )
-  )
+    use_dump_check(list(), 100000)
+  ) %>%
+    expect_warning()
 
   # query entries are not all NULL and rows > 99999
   expect_true(
-    suppressWarnings(
-      phsopendata:::use_dump_check(list(q = 4), 100000)
-    )
-  )
+    use_dump_check(list(q = 4), 100000)
+  ) %>%
+    expect_warning()
 
   expect_true(
-    suppressWarnings(
-      phsopendata:::use_dump_check(list(q = 4), 100000)
-    )
-  )
+    use_dump_check(list(q = 4), 100000)
+  ) %>%
+    expect_warning()
 })
 
 test_that("returns false as expected", {
   # rows is NULL and query list is not all NULL
   expect_false(
-    phsopendata:::use_dump_check(list(fields = "Age"), NULL)
+    use_dump_check(list(fields = "Age"), NULL)
   )
 
   # rows is under 99999 and query list is not all NULL
   expect_false(
-    phsopendata:::use_dump_check(list(fields = "Age"), 100)
+    use_dump_check(list(fields = "Age"), 100)
   )
 
   # rows is under 99999 and query list is all NULL
   expect_false(
-    phsopendata:::use_dump_check(list(), 100)
+    use_dump_check(list(), 100)
   )
 })