From a3145f6fa4fd91c4cefb4870944c7d0ce6ae8d0f Mon Sep 17 00:00:00 2001
From: Rich FitzJohn <r.fitzjohn@imperial.ac.uk>
Date: Thu, 20 Jul 2023 18:21:59 +0100
Subject: [PATCH] Expand testing

---
 R/cleanup.R                               |  4 +++-
 R/metadata.R                              |  7 +------
 R/root.R                                  |  3 ---
 R/util.R                                  | 10 +++++-----
 tests/testthat/test-interactive-cleanup.R | 12 ++++++++++++
 tests/testthat/test-util.R                |  8 ++++++++
 6 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/R/cleanup.R b/R/cleanup.R
index d5f670b6..966b3cad 100644
--- a/R/cleanup.R
+++ b/R/cleanup.R
@@ -53,7 +53,9 @@ orderly_cleanup_status <- function(name = NULL, root = NULL, locate = TRUE) {
   p <- get_active_packet()
   is_active <- !is.null(p)
   if (is_active) {
-    stop("Don't call this with an acive packet...")
+    cli::cli_abort(
+      "Don't call 'orderly2::orderly_cleanup_status()' from a running packet",
+      i = "The orderly_cleanup* functions are for interactive use only")
   }
 
   if (is.null(name)) {
diff --git a/R/metadata.R b/R/metadata.R
index faf4e249..2efeb6c7 100644
--- a/R/metadata.R
+++ b/R/metadata.R
@@ -377,12 +377,7 @@ copy_global <- function(path_root, path_dest, config, files) {
 
 
 static_orderly_global_resource <- function(args) {
-  files <- lapply(args, static_character_vector, TRUE)
-  if (length(files) == 0 || !all(lengths(files) == 1)) {
-    NULL
-  } else {
-    list_to_character(files)
-  }
+  unlist(lapply(args, static_character_vector, TRUE), FALSE, TRUE)
 }
 
 
diff --git a/R/root.R b/R/root.R
index c54f9d38..3700943e 100644
--- a/R/root.R
+++ b/R/root.R
@@ -70,9 +70,6 @@ orderly_init <- function(path, ...) {
 ## * database - lower priority, as only VIMC and everything else must work first
 ## * minimum_orderly_version - the required version
 orderly_root <- function(root, locate) {
-  if (inherits(root, "orderly_root")) {
-    return(root)
-  }
   root <- outpack_root_open(root, locate)
   ## NOTE: it's can't be changed yet, but core.path_archive cannot be
   ## "draft" for this to work well.
diff --git a/R/util.R b/R/util.R
index 3f3ce352..4d5aff63 100644
--- a/R/util.R
+++ b/R/util.R
@@ -390,13 +390,13 @@ check_symbol_from_str <- function(str, name) {
 }
 
 
-
+## Currently supported in gert 1.9000 (which is the 2.0
+## prerelease). Rather than depend on that version directly, requiring
+## people to install it from source or r-universe, just sniff for the
+## function we need, for now:
 gert_git_ignore_path_is_ignored <- function() {
-  not_supported <- function(path, repo) {
-    rep_len(NA, length(path))
-  }
   tryCatch(getExportedValue("gert", "git_ignore_path_is_ignored"),
-             error = function(e) not_supported)
+             error = function(e) NULL)
 }
 
 
diff --git a/tests/testthat/test-interactive-cleanup.R b/tests/testthat/test-interactive-cleanup.R
index 2db3ca04..f3696e1d 100644
--- a/tests/testthat/test-interactive-cleanup.R
+++ b/tests/testthat/test-interactive-cleanup.R
@@ -144,3 +144,15 @@ test_that("can clean up directories", {
     dir(path_src, recursive = TRUE, include.dirs = TRUE),
     c("data", "data/a.csv", "data/b.csv", "orderly.R"))
 })
+
+
+test_that("Don't call cleanup on an active packet", {
+  path <- test_prepare_orderly_example("data")
+  path_src <- file.path(path, "src", "data")
+  append_lines(file.path(path_src, "orderly.R"),
+               "orderly2::orderly_cleanup_status()")
+  expect_error(
+    orderly_run("data", root = path),
+    "Don't call 'orderly2::orderly_cleanup_status()' from a running packet",
+    fixed = TRUE)
+})
diff --git a/tests/testthat/test-util.R b/tests/testthat/test-util.R
index 76eea38f..4b728059 100644
--- a/tests/testthat/test-util.R
+++ b/tests/testthat/test-util.R
@@ -190,3 +190,11 @@ test_that("can check two vars are of same type", {
   expect_false(is_same_type(1, "TRUE"))
   expect_false(is_same_type(TRUE, "TRUE"))
 })
+
+
+test_that("fall back if gert does not support ignored files", {
+  mock_ns <- mockery::mock(identity, stop("not found"))
+  mockery::stub(gert_git_ignore_path_is_ignored, "getExportedValue", mock_ns)
+  expect_equal(gert_git_ignore_path_is_ignored(), identity)
+  expect_null(gert_git_ignore_path_is_ignored())
+})