Merge pull request #171 from mrc-ide/mrc-5735-delete-implicit

Improve handling of deleted implicit inputs.

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: orderly2
 Title: Orderly Next Generation
-Version: 1.99.30
+Version: 1.99.31
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
                     email = "[email protected]"),
              person("Robert", "Ashton", role = "aut"),

R/run.R

@@ -430,29 +430,33 @@ check_files_strict <- function(path, known, artefacts) {
   all_files_end <- dir_ls_local(path, all = TRUE, type = "file")
   unknown <- setdiff(all_files_end, all_known)
   if (length(unknown) > 0) {
-    ## TODO: better once we have logging merged, I think
-    ##
     ## TODO: provide workaround to ignore too (either to exclude
     ## or ignore); see VIMC-7093
-    message(paste(
-      "orderly produced unexpected files:",
-      sprintf("  - %s", unknown),
-      "Consider using orderly2::orderly_artefact() to describe them",
-      sep = "\n"))
+    cli::cli_alert_warning("Report produced unexpected files:")
+    cli::cli_ul(unknown)
+    cli::cli_alert_info(paste("These are probably artefacts, consider using",
+                              "orderly2::orderly_artefact to describe them"))
   }
 }
 
 
 check_files_relaxed <- function(path, inputs_info) {
   inputs_info_end <- withr::with_dir(path, fs::file_info(inputs_info$path))
-  i <- inputs_info_end$size != inputs_info$size |
-    inputs_info_end$modification_time != inputs_info$modification_time
-  if (any(i)) {
-    message(paste(
-      "inputs modified; these are probably artefacts:",
-      sprintf("  - %s", inputs_info$path[i]),
-      "Consider using orderly2::orderly_artefact() to describe them",
-      sep = "\n"))
+  deleted <- is.na(inputs_info_end$type)
+
+  modified <- !deleted &
+    (inputs_info_end$size != inputs_info$size |
+     inputs_info_end$modification_time != inputs_info$modification_time)
+
+  if (any(modified)) {
+    cli::cli_alert_warning("The following inputs were modified by the report:")
+    cli::cli_ul(inputs_info$path[modified])
+    cli::cli_alert_info(paste("These are probably artefacts, consider using",
+                              "orderly2::orderly_artefact to describe them"))
+  }
+  if (any(deleted)) {
+    cli::cli_alert_warning("The following inputs were deleted by the report:")
+    cli::cli_ul(inputs_info$path[deleted])
   }
 }
 

tests/testthat/test-run.R

@@ -86,14 +86,12 @@ test_that("raises deprecation warning for orderly.R", {
   path <- test_prepare_orderly_example("deprecated-orderly-name")
   envir <- new.env()
   rlang::reset_warning_verbosity("deprecate_orderly_file_name")
-  suppressMessages(
-    expect_warning(
-      orderly_run("deprecated-orderly-name", root = path,
-                  envir = envir, echo = FALSE),
-      paste("Naming convention orderly.R will be deprecated",
-            "soon. Please change orderly file name to",
-            "<reportname>.R")
-    )
+
+  expect_warning(
+    orderly_run_quietly("deprecated-orderly-name", root = path, envir = envir),
+    paste("Naming convention orderly.R will be deprecated",
+          "soon. Please change orderly file name to",
+          "<reportname>.R")
   )
 })
 
@@ -435,9 +433,10 @@ test_that("with strict mode, indicate unknown files as potential artefacts", {
              path_src)
   res <- testthat::evaluate_promise(
     id <- orderly_run("implicit", root = path, envir = new.env()))
-  expect_match(res$messages,
-               "orderly produced unexpected files:\n  - mygraph.png",
-               all = FALSE)
+
+  expect_match(res$messages, "Report produced unexpected files:", all = FALSE)
+  expect_match(res$messages, "mygraph.png", all = FALSE)
+
   expect_setequal(
     dir(file.path(path, "archive", "implicit", id)),
     c("implicit.R", "mygraph.png", "data.csv"))
@@ -449,16 +448,36 @@ test_that("without strict mode, detect modified files", {
   file.create(file.path(path, "src", "implicit", "mygraph.png"))
   res <- testthat::evaluate_promise(
     id <- orderly_run("implicit", root = path, envir = new.env()))
-  expect_match(
-    res$messages,
-    "inputs modified; these are probably artefacts:\n  - mygraph.png",
-    fixed = TRUE, all = FALSE)
+
+  expect_match(res$messages, "inputs were modified by the report:", all = FALSE)
+  expect_match(res$messages, "mygraph.png", all = FALSE, fixed = TRUE)
+
   expect_setequal(
     dir(file.path(path, "archive", "implicit", id)),
     c("implicit.R", "mygraph.png", "data.csv"))
 })
 
 
+test_that("without strict mode, detect deleted files", {
+  path <- create_temporary_root()$path
+  path_src <- file.path(path, "src", "report")
+
+  fs::dir_create(path_src)
+  writeLines("0,1,2,3", file.path(path_src, "data.csv"))
+  writeLines("fs::file_delete('data.csv')", file.path(path_src, "report.R"))
+
+  res <- testthat::evaluate_promise(
+    id <- orderly_run("report", root = path, envir = new.env()))
+
+  expect_match(res$messages, "inputs were deleted by the report:", all = FALSE)
+  expect_match(res$messages, "data.csv", all = FALSE, fixed = TRUE)
+
+  expect_setequal(
+    dir(file.path(path, "archive", "report", id)),
+    c("report.R"))
+})
+
+
 test_that("disallow multiple calls to strict mode", {
   path <- test_prepare_orderly_example("implicit")
   path_src <- file.path(path, "src", "implicit", "implicit.R")
@@ -1389,7 +1408,6 @@ test_that("warn if description unnamed in artefact", {
   writeLines(sub("description = ", "", code), path_src)
   envir <- new.env()
   expect_warning(
-    suppressMessages(
-      orderly_run("data", root = path, envir = envir, echo = FALSE)),
+    orderly_run_quietly("data", root = path, envir = envir),
     "Please use a named argument")
 })