From 4caad52a2ae84f3e10259be6a3ffd6339bd693ef Mon Sep 17 00:00:00 2001
From: shikokuchuo <53399081+shikokuchuo@users.noreply.github.com>
Date: Wed, 28 Feb 2024 08:40:39 +0000
Subject: [PATCH] Make consistent assert_package() and update test suite

---
 R/assert_package.R          | 13 ++++++-------
 R/build_universe.R          |  7 +------
 R/utils_assert.R            |  2 +-
 tests/test-assert_package.R | 38 ++++++++-----------------------------
 4 files changed, 16 insertions(+), 44 deletions(-)

diff --git a/R/assert_package.R b/R/assert_package.R
index 8e5bcb9..09d29e4 100644
--- a/R/assert_package.R
+++ b/R/assert_package.R
@@ -6,15 +6,15 @@
 #'   otherwise `NULL` if there are no issues.
 assert_package <- function(path) {
   if (!is_character_scalar(path)) {
-    return("Invalid package file path.")
+    stop("Invalid package file path")
   }
   if (!file.exists(path)) {
-    return(paste("file", shQuote(path), "does not exist."))
+    stop(paste("file", shQuote(path), "does not exist"))
   }
   name <- basename(path)
   url <- try(readLines(path, warn = FALSE), silent = TRUE)
   if (inherits(url, "try-error")) {
-    return(paste("Problem reading file", shQuote(path)))
+    stop(paste("Problem reading file", shQuote(path)))
   }
   assert_package_contents(name = name, url = url)
 }
@@ -25,19 +25,18 @@ assert_package_contents <- function(name, url) {
     x = name
   )
   if (!isTRUE(good_package_name)) {
-    return(paste("Found invalid package name: ", shQuote(name)))
+    stop(paste("Found invalid package name: ", shQuote(name)))
   }
   if (!is_character_scalar(url)) {
-    return("Invalid package URL.")
+    stop("Invalid package URL")
   }
   good_url <- grepl(
     pattern = "^https?://[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,3}(/\\S*)?$",
     x = url
   )
   if (!isTRUE(good_url)) {
-    return(
+    stop(
       paste("Found malformed URL", shQuote(url), "of package", shQuote(name))
     )
   }
-  NULL
 }
diff --git a/R/build_universe.R b/R/build_universe.R
index aa40cd0..25b8344 100644
--- a/R/build_universe.R
+++ b/R/build_universe.R
@@ -13,12 +13,7 @@ build_universe <- function(input = getwd(), output = "packages.json") {
   assert_character_scalar(output, "invalid output")
   assert_file(input)
   packages <- list.files(input, all.files = FALSE, full.names = TRUE)
-  for (package in packages) {
-    result <- assert_package(package)
-    if (!is.null(result)) {
-      stop(result, call. = FALSE)
-    }
-  }
+  lapply(packages, assert_package)
   urls <- vapply(
     X = packages,
     FUN = readLines,
diff --git a/R/utils_assert.R b/R/utils_assert.R
index 2809a23..05cd20f 100644
--- a/R/utils_assert.R
+++ b/R/utils_assert.R
@@ -1,6 +1,6 @@
 assert_file <- function(x) {
   if (!file.exists(x)) {
-    stop("File ", shQuote(x), " does not exist.", call. = FALSE)
+    stop("File ", shQuote(x), " does not exist", call. = FALSE)
   }
 }
 
diff --git a/tests/test-assert_package.R b/tests/test-assert_package.R
index 3e3cff9..bf3a638 100644
--- a/tests/test-assert_package.R
+++ b/tests/test-assert_package.R
@@ -1,48 +1,26 @@
-stopifnot(
-  grepl(
-    "Invalid package file path",
-    r.releases.utils::assert_package(path = c(1L, 2L))
-  )
-)
+expect_error <- function(x, e = "") {
+  invisible(grepl(e, tryCatch(x, error = identity)[["message"]], fixed = TRUE) || stop("Error '", e, "' not generated"))
+}
 
-stopifnot(
-  grepl(
-    "does not exist",
-    r.releases.utils::assert_package(path = tempfile())
-  )
-)
+expect_error(r.releases.utils::assert_package(path = c(1L, 2L)), "Invalid package file path")
+expect_error(r.releases.utils::assert_package(path = tempfile()), "does not exist")
 
 path <- file.path(tempfile(), "hy-phens")
 dir.create(dirname(path))
 file.create(path)
-stopifnot(
-  grepl(
-    "invalid package name",
-    r.releases.utils::assert_package(path = path)
-  )
-)
+expect_error(r.releases.utils::assert_package(path = path), "invalid package name")
 unlink(dirname(path), recursive = TRUE)
 
 path <- file.path(tempfile(), "package")
 dir.create(dirname(path))
 writeLines(letters, path)
-stopifnot(
-  grepl(
-    "Invalid package URL",
-    r.releases.utils::assert_package(path = path)
-  )
-)
+expect_error(r.releases.utils::assert_package(path = path), "Invalid package URL")
 unlink(dirname(path), recursive = TRUE)
 
 path <- file.path(tempfile(), "package")
 dir.create(dirname(path))
 writeLines("b a d", path)
-stopifnot(
-  grepl(
-    "Found malformed URL",
-    r.releases.utils::assert_package(path = path)
-  )
-)
+expect_error(r.releases.utils::assert_package(path = path), "Found malformed URL")
 unlink(dirname(path), recursive = TRUE)
 
 path <- file.path(tempfile(), "package")