From 232b9fe98ea46fc36e7b2cfe2f88de6dea9a4bff Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Fri, 29 Sep 2023 15:48:42 +0100 Subject: [PATCH] Rename and reorganise --- R/config.R | 4 ++-- R/metadata.R | 12 ++++++------ R/outpack_metadata.R | 2 +- R/outpack_packet.R | 4 ++-- R/read.R | 4 ++-- R/util_assert.R | 24 ++++++++++++------------ tests/testthat/helper-outpack.R | 1 - tests/testthat/test-util-assert.R | 23 +++++++++++------------ 8 files changed, 36 insertions(+), 38 deletions(-) diff --git a/R/config.R b/R/config.R index 8801b13f..f8c2f7a5 100644 --- a/R/config.R +++ b/R/config.R @@ -1,7 +1,7 @@ orderly_config_read <- function(path, call = NULL) { filename <- file.path(path, "orderly_config.yml") - assert_file_exists2(basename(filename), workdir = path, - name = "Orderly configuration", call) + assert_file_exists_relative(basename(filename), workdir = path, + name = "Orderly configuration", call = call) raw <- yaml_read(filename) if (!is.null(raw)) { diff --git a/R/metadata.R b/R/metadata.R index a7b7dff5..25113d60 100644 --- a/R/metadata.R +++ b/R/metadata.R @@ -151,8 +151,8 @@ static_orderly_description <- function(args) { orderly_resource <- function(files) { p <- get_active_packet() src <- if (is.null(p)) "." else p$orderly2$src - assert_file_exists2(files, workdir = src, name = "Resource file", - call = environment()) + assert_file_exists_relative(files, workdir = src, name = "Resource file", + call = environment()) files_expanded <- expand_dirs(files, src) if (!is.null(p)) { if (p$orderly2$strict$enabled) { @@ -161,8 +161,8 @@ orderly_resource <- function(files) { ## Above we're looking in the underlying source directory, here ## we're looking within the running directory; it's not obvious ## when this second case would fail, really. - assert_file_exists2(files, workdir = p$path, name = "Resource file", - call = environment()) + assert_file_exists_relative(files, workdir = p$path, + name = "Resource file", call = environment()) } outpack_packet_file_mark(p, files_expanded, "immutable") p$orderly2$resources <- c(p$orderly2$resources, files_expanded) @@ -358,8 +358,8 @@ copy_shared_resource <- function(path_root, path_dest, config, files, call) { here <- names(files) there <- unname(files) - assert_file_exists2(there, workdir = shared_path, name = "Shared resource", - call = call) + assert_file_exists_relative(there, workdir = shared_path, + name = "Shared resource file", call = call) src <- file.path(shared_path, there) dst <- file.path(path_dest, here) diff --git a/R/outpack_metadata.R b/R/outpack_metadata.R index 349d77a5..3691735d 100644 --- a/R/outpack_metadata.R +++ b/R/outpack_metadata.R @@ -76,7 +76,7 @@ outpack_metadata_create <- function(path, name, id, time, files, if (is.null(files)) { files <- dir(path, recursive = TRUE, all.files = TRUE, no.. = TRUE) } else { - assert_file_exists2(files, name = "File", workdir = path) + assert_file_exists_relative(files, name = "File", workdir = path) } if (length(file_ignore) > 0) { diff --git a/R/outpack_packet.R b/R/outpack_packet.R index f42b5c55..ff422b18 100644 --- a/R/outpack_packet.R +++ b/R/outpack_packet.R @@ -325,8 +325,8 @@ outpack_packet_file_mark <- function(packet, files, status) { status <- match_value(status, c("immutable", "ignored")) packet <- check_current_packet(packet) - assert_file_exists2(files, workdir = packet$path, name = "File", - call = environment()) + assert_file_exists_relative(files, workdir = packet$path, name = "File", + call = environment()) ## TODO: these are exclusive categories because we later return a ## 1:1 mapping of file to status diff --git a/R/read.R b/R/read.R index c1d1a177..99fe674d 100644 --- a/R/read.R +++ b/R/read.R @@ -1,6 +1,6 @@ orderly_read <- function(path, call = NULL) { - assert_file_exists2("orderly.R", name = "Orderly file", workdir = path, - call = call) + assert_file_exists_relative("orderly.R", name = "Orderly file", + workdir = path, call = call) orderly_read_r(file.path(path, "orderly.R")) } diff --git a/R/util_assert.R b/R/util_assert.R index 28372ea2..d67c3171 100644 --- a/R/util_assert.R +++ b/R/util_assert.R @@ -51,17 +51,18 @@ assert_is <- function(x, what, name = deparse(substitute(x))) { } } -assert_file_exists <- function(x, workdir = NULL, name = "File") { - err <- !file_exists(x, workdir = workdir) +assert_file_exists <- function(files, name = "File", call = NULL) { + err <- !file.exists(files) if (any(err)) { - msg <- squote(x[err]) - stop(sprintf("%s does not exist: %s", name, paste(msg, collapse = ", ")), - call. = FALSE) + n <- cli::qty(sum(err)) + cli::cli_abort( + "{name}{n}{?s} {?does/do} not exist: {collapseq(files[err])}", + call = call) } } -assert_file_exists2 <- function(files, workdir, name, call = NULL) { +assert_file_exists_relative <- function(files, workdir, name, call = NULL) { assert_relative_path(files, name, workdir, call) assert_character(files, call = call) @@ -93,13 +94,12 @@ assert_file_exists2 <- function(files, workdir, name, call = NULL) { } } -assert_is_directory <- function(x, workdir = NULL, name = "Directory") { - assert_file_exists(x, workdir, name) - path <- if (is.null(workdir)) x else file.path(workdir, x) +assert_is_directory <- function(path, name = "Directory", call = NULL) { + assert_scalar_character(path) + assert_file_exists(path, name = name, call = call) if (!is_directory(path)) { - stop(sprintf("Path exists but is not a directory: %s", - paste(x, collapse = ", ")), - call. = FALSE) + cli::cli_abort("Path exists but is not a directory: {path}", + call = call) } } diff --git a/tests/testthat/helper-outpack.R b/tests/testthat/helper-outpack.R index ff2dbe2f..12eff2a5 100644 --- a/tests/testthat/helper-outpack.R +++ b/tests/testthat/helper-outpack.R @@ -161,7 +161,6 @@ outpack_packet_run <- function(packet, script, envir = NULL) { envir <- new.env(parent = .GlobalEnv) } packet <- check_current_packet(packet) - assert_file_exists(script, workdir = packet$path, name = "Script") withr::with_dir(packet$path, source_echo(script, envir = envir, echo = FALSE)) } diff --git a/tests/testthat/test-util-assert.R b/tests/testthat/test-util-assert.R index 317905c2..98dfd017 100644 --- a/tests/testthat/test-util-assert.R +++ b/tests/testthat/test-util-assert.R @@ -44,30 +44,29 @@ test_that("assert_is", { test_that("assert_file_exists", { tmp <- normalise_path(tempdir()) path <- tempfile(tmpdir = tmp) - expect_error(assert_file_exists(path), "File does not exist") + expect_error(assert_file_exists(path, "File"), "File does not exist") file.create(path) - expect_silent(assert_file_exists(path)) - expect_silent(assert_file_exists(basename(path), workdir = tmp)) + expect_silent(assert_file_exists(path, "File")) }) -test_that("assert_file_exists2 works checks if files exist", { +test_that("assert_file_exists_relative works checks if files exist", { tmp <- withr::local_tempdir() file.create(file.path(tmp, "c")) - expect_error(assert_file_exists2("a", tmp, "File"), + expect_error(assert_file_exists_relative("a", tmp, "File"), "File does not exist: 'a'") - expect_error(assert_file_exists2(c("a", "b"), tmp, "File"), + expect_error(assert_file_exists_relative(c("a", "b"), tmp, "File"), "Files do not exist: 'a', 'b'") - expect_error(assert_file_exists2(c("a", "b", "c", "d"), tmp, "File"), + expect_error(assert_file_exists_relative(c("a", "b", "c", "d"), tmp, "File"), "Files do not exist: 'a', 'b', 'd'") - expect_silent(assert_file_exists2("c", tmp, "File")) + expect_silent(assert_file_exists_relative("c", tmp, "File")) }) -test_that("assert_file_exists2 informs about case mismatch", { +test_that("assert_file_exists_relative informs about case mismatch", { testthat::skip_if_not_installed("mockery") mock_file_exists <- mockery::mock(TRUE, cycle = TRUE) - mockery::stub(assert_file_exists2, "file_exists", mock_file_exists) + mockery::stub(assert_file_exists_relative, "file_exists", mock_file_exists) tmp <- withr::local_tempdir() file.create(file.path(tmp, "a")) @@ -75,7 +74,7 @@ test_that("assert_file_exists2 informs about case mismatch", { file.create(file.path(tmp, "b/c/d")) err <- expect_error( - assert_file_exists2("A", tmp, "File"), + assert_file_exists_relative("A", tmp, "File"), "File does not exist: 'A'") expect_length(err$body, 3) expect_equal(names(err$body), c("i", "i", "i")) @@ -84,7 +83,7 @@ test_that("assert_file_exists2 informs about case mismatch", { expect_match(err$body[[3]], "Looked within directory '.+'") err <- expect_error( - assert_file_exists2(c("A", "b/C/d"), tmp, "File"), + assert_file_exists_relative(c("A", "b/C/d"), tmp, "File"), "Files do not exist: 'A', 'b/C/d'") expect_length(err$body, 4) expect_equal(names(err$body), c("i", "i", "i", "i"))