From 0db1d2ad8f29209e548a46bd2d5274ac3fcc4cb4 Mon Sep 17 00:00:00 2001 From: Paul Lietar Date: Mon, 2 Sep 2024 16:42:53 +0100 Subject: [PATCH 1/2] Improve handling of deleted implicit inputs. In implicit mode, orderly prints a message if some of the implicit inputs have been modified by the report. However the code to enumerate modified files did not properly handle files that have been deleted. When that happens, `fs::file_info` return NA in all the metadata columns. We now detect these files and display a slightly different warning for it. Cleaned up a bit of adjacent code: switch from handcrafted bullet points to using `cli::cli_ul`, fixed some tests which had used `suppressMessages` when they could have used `orderly_run_quietly`. --- R/run.R | 34 ++++++++++++++----------- tests/testthat/test-run.R | 52 ++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/R/run.R b/R/run.R index f6815568..10c1b6d4 100644 --- a/R/run.R +++ b/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]) } } diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index a514a96a..05fd7050 100644 --- a/tests/testthat/test-run.R +++ b/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", - ".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", + ".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") }) From 1e1f24e7e8b9408ac50ad65ddb09b63e771addc3 Mon Sep 17 00:00:00 2001 From: Paul Lietar Date: Mon, 2 Sep 2024 23:11:38 +0100 Subject: [PATCH 2/2] Bump version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8cb4681c..a3e84498 100644 --- a/DESCRIPTION +++ b/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 = "rich.fitzjohn@gmail.com"), person("Robert", "Ashton", role = "aut"),