refactor: remove prefix from connect() and just use the usual env vars (#477)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: nealrichardson

.github/workflows/integration-tests.yaml

@@ -47,17 +47,11 @@ jobs:
         shell: Rscript {0}
 
       - name: Run integration tests
-        uses: nealrichardson/with-connect
+        uses: posit-dev/with-connect@main
         env:
           CONNECTAPI_INTEGRATED: "true"
         with:
           version: ${{ matrix.version }}
           license: ${{ secrets.CONNECT_LICENSE_FILE }}
-          # TODO: rewrite tests to use CONNECT_* env vars directly
           command: |
-            Rscript -e '
-              Sys.setenv(
-                TEST_1_SERVER=Sys.getenv("CONNECT_SERVER"),
-                TEST_1_API_KEY=Sys.getenv("CONNECT_API_KEY")
-              )
-              source("tests/test-integrated.R")'
+            Rscript -e 'source("tests/test-integrated.R")'

.github/workflows/pkgdown.yaml

@@ -34,18 +34,12 @@ jobs:
           needs: website
 
       - name: Build site
-        uses: nealrichardson/with-connect
+        uses: posit-dev/with-connect@main
         with:
           # Runs on the default version in the with-connect action
           license: ${{ secrets.CONNECT_LICENSE_FILE }}
-          # TODO: rewrite tests to use CONNECT_* env vars directly
           command: |
-            Rscript -e '
-              Sys.setenv(
-                TEST_1_SERVER=Sys.getenv("CONNECT_SERVER"),
-                TEST_1_API_KEY=Sys.getenv("CONNECT_API_KEY")
-              )
-              pkgdown::build_site_github_pages(new_process = FALSE, install = FALSE)'
+            Rscript -e 'pkgdown::build_site_github_pages(new_process = FALSE, install = FALSE)'
 
       - name: Deploy to GitHub pages 🚀
         if: github.event_name != 'pull_request'

.github/workflows/test-coverage.yaml

@@ -29,25 +29,19 @@ jobs:
           needs: coverage
 
       - name: Run coverage
-        uses: nealrichardson/with-connect
+        uses: posit-dev/with-connect@main
         env:
           CONNECTAPI_INTEGRATED: "true"
         with:
           # Runs on the default version in the with-connect action
           license: ${{ secrets.CONNECT_LICENSE_FILE }}
-          # TODO: rewrite tests to use CONNECT_* env vars directly
           command: |
             Rscript -e '
-              Sys.setenv(
-                TEST_1_SERVER=Sys.getenv("CONNECT_SERVER"),
-                TEST_1_API_KEY=Sys.getenv("CONNECT_API_KEY")
-              )
-              token <- "${{ secrets.CODECOV_TOKEN }}"
               covr::codecov(
                 quiet = FALSE,
                 clean = FALSE,
                 install_path = file.path(normalizePath(Sys.getenv("RUNNER_TEMP"), winslash = "/"), "package"),
-                token = if (token != "") token
+                token = "${{ secrets.CODECOV_TOKEN }}"
               )'
 
 

NEWS.md

@@ -1,5 +1,7 @@
 # connectapi (development version)
 
+- Removed `prefix` argument to `connect()` function, which was a convenience utility for testing. (#477)
+
 # connectapi 0.9.0
 
 - New `set_integrations()` function to set the OAuth integration

R/connect.R

@@ -963,7 +963,6 @@ Connect <- R6::R6Class(
 #'   `Connect` object using the `api_key`. By providing a different
 #'   key here you can test a visitor client with differently-scoped
 #'   permissions.
-#' @param prefix The prefix used to determine environment variables
 #' @param audience Optional. The GUID of a Connect API integration associated with this piece of content.
 #' @param ... Additional arguments. Not used at present
 #' @param .check_is_fatal Whether to fail if "check" requests fail. Useful in
@@ -991,21 +990,21 @@ Connect <- R6::R6Class(
 #' @examplesIf identical(Sys.getenv("IN_PKGDOWN"), "true")
 #'
 #' # default is to read CONNECT_SERVER and CONNECT_API_KEY environment variables
-#' # this example will read TEST_1_SERVER and TEST_1_API_KEY
-#' connect(prefix = "TEST_1")
+#' connect()
 #'
 #' @export
 connect <- function(
-  server = Sys.getenv(paste0(prefix, "_SERVER"), NA_character_),
-  api_key = Sys.getenv(paste0(prefix, "_API_KEY"), NA_character_),
+  server = Sys.getenv("CONNECT_SERVER"),
+  api_key = Sys.getenv("CONNECT_API_KEY"),
   token,
   token_local_testing_key = api_key,
-  prefix = "CONNECT",
   audience = NULL,
   ...,
   .check_is_fatal = TRUE
 ) {
-  if (is.null(api_key) || is.na(api_key) || nchar(api_key) == 0) {
+  if (
+    missing(token) && is.null(api_key) || is.na(api_key) || nchar(api_key) == 0
+  ) {
     msg <- "Invalid (empty) API key. Please provide a valid API key"
     if (.check_is_fatal) {
       stop(glue::glue("ERROR: {msg}"))

R/deploy.R

@@ -402,7 +402,7 @@ download_bundle <- function(
 #' }
 #' @examplesIf identical(Sys.getenv("IN_PKGDOWN"), "true")
 #'
-#' client <- connect(prefix = "TEST_1")
+#' client <- connect()
 #' bnd <- bundle_path(system.file("tests/testthat/examples/static.tar.gz", package = "connectapi"))
 #' deploy(client, bnd)
 #'
@@ -413,8 +413,7 @@ deploy <- function(
   title = name,
   guid = NULL,
   ...,
-  .pre_deploy = {
-  }
+  .pre_deploy = {}
 ) {
   validate_R6_class(bundle, "Bundle")
   validate_R6_class(connect, "Connect")

man/connect.Rd

@@ -1 +1 @@
-test_conn_1 <- connect(prefix = "TEST_1")
+client <- connect()

man/deploy.Rd

@@ -1,78 +1,12 @@
-test_that("connect works", {
-  conn <- connect(
-    server = Sys.getenv("TEST_1_SERVER"),
-    api_key = Sys.getenv("TEST_1_API_KEY")
-  )
-  expect_true(validate_R6_class(conn, "Connect"))
-})
-
-test_that("connect works with prefix only", {
-  conn <- connect(prefix = "TEST_1")
-  expect_true(validate_R6_class(conn, "Connect"))
-})
-
-test_that("connect fails for nonexistent server", {
-  expect_error({
-    connect(server = "does-not-exist.rstudio.com", api_key = "bogus")
-  })
+# This whole suite assumes CONNECT_SERVER and CONNECT_API_KEY env vars are set
+test_that("connect() works", {
+  expect_true(validate_R6_class(connect(), "Connect"))
 })
 
 test_that("connect fails for good server, bad api key", {
   expect_error({
     connect(
-      server = Sys.getenv("TEST_1_SERVER"),
       api_key = "bogus"
     )
   })
 })
-
-test_that("error if API key is empty", {
-  expect_error(
-    connect(server = Sys.getenv("TEST_1_SERVER"), api_key = ""),
-    "provide a valid API key"
-  )
-
-  expect_error(
-    connect(server = Sys.getenv("TEST_1_SERVER"), api_key = NA_character_),
-    "provide a valid API key"
-  )
-
-  expect_error(
-    connect(server = Sys.getenv("TEST_1_SERVER"), api_key = NULL),
-    "provide a valid API key"
-  )
-})
-
-test_that(".check_is_fatal toggle works", {
-  expect_error(
-    connect(server = Sys.getenv("TEST_1_SERVER"), api_key = ""),
-    "provide a valid API key"
-  )
-
-  rsc <- connect(
-    server = Sys.getenv("TEST_1_SERVER"),
-    api_key = "",
-    .check_is_fatal = FALSE
-  )
-  expect_true(
-    validate_R6_class(rsc, "Connect")
-  )
-
-  expect_error(
-    suppressMessages(connect(
-      server = "http://fake-value.example.com",
-      api_key = "fake-value"
-    )),
-    "Could not resolve host"
-  )
-
-  # TODO: suppressing the message in the tryCatch handler does not work...?
-  rsc1 <- suppressMessages(connect(
-    server = "http://fake-value.example.com",
-    api_key = "fake-value",
-    .check_is_fatal = FALSE
-  ))
-  expect_true(
-    validate_R6_class(rsc1, "Connect")
-  )
-})