fix: Say CONNECT_API_KEY is missing instead of erroring on NA (0.8.6)

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: audit.connect
 Title: Posit Connect Health Check
-Version: 0.8.5
+Version: 0.8.6
 Authors@R:
     person("Jumping", "Rivers", , "info@jumpingrivers.com", role = c("aut", "cre"))
 Description: Posit Connect Health Check. Deploys various content types to
@@ -32,6 +32,7 @@ Suggests:
     gtExtras,
     htmltools,
     testthat (>= 3.0.0),
+    withr,
     yaml
 Remotes:
     jumpingrivers/audit.base,

NEWS.md

@@ -1,3 +1,9 @@
+# audit.connect 0.8.6
+
+## Bug fixes and minor improvements
+
+* `check()` says "CONNECT_API_KEY missing" when the key is unset, instead of "missing value where TRUE/FALSE needed" (#71).
+
 # audit.connect 0.8.5
 
 ## New features

R/set_key.R

@@ -1,9 +1,13 @@
+is_missing = function(value) {
+  length(value) != 1L || is.na(value) || !nzchar(value)
+}
+
 set_key = function(name, value) {
-  .connect[[name]] = value
-  if (is.null(value) || nchar(value) == 0L) {
+  if (is_missing(value)) {
     error_msg = paste0("Missing ", name)
     stop(error_msg, call. = FALSE)
   }
+  .connect[[name]] = value
   invisible(value)
 }
 

R/summarise_setup.R

@@ -55,10 +55,11 @@ check_server_accessibility = function(server) {
 }
 
 check_api_key = function(token) {
-  value = set_key("connect_api_key", get_value("CONNECT_API_KEY", token))
-  if (is.na(value)) {
+  value = get_value("CONNECT_API_KEY", token)
+  if (is_missing(value)) {
     cli::cli_abort("CONNECT_API_KEY missing")
   }
+  set_key("connect_api_key", value)
   cli::cli_alert_info("API KEY: {cli::col_green(cli::symbol$tick)}")
   invisible(TRUE)
 }

tests/testthat/test-set_key.R

@@ -0,0 +1,16 @@
+test_that("set_key() rejects missing values without storing them", {
+  withr::defer(rm(list = intersect("test_key", ls(.connect)), envir = .connect))
+  for (value in list(NULL, NA_character_, "", character(0))) {
+    expect_error(set_key("test_key", value), "Missing test_key")
+  }
+  expect_null(.connect[["test_key"]])
+  expect_identical(set_key("test_key", "abc"), "abc")
+  expect_identical(.connect[["test_key"]], "abc")
+})
+
+test_that("check_api_key() says when CONNECT_API_KEY is missing", {
+  withr::local_envvar(CONNECT_API_KEY = NA)
+  expect_error(check_api_key(NULL), "CONNECT_API_KEY missing")
+  withr::local_envvar(CONNECT_API_KEY = "")
+  expect_error(check_api_key(NULL), "CONNECT_API_KEY missing")
+})