From cf3eb8bbdba3c9ad5aa45cfffd3d0d86e0aeb7fa Mon Sep 17 00:00:00 2001 From: Ilya Zarubin <36898027+ilyaZar@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:12:09 +0200 Subject: [PATCH] Fix 1074 to improve code coverage of `R/utils.R` to 100% (#1075) * feature: improve is_exisiting_module() - make it self-check if is called inside an R package (by checking for existence of an R-directory) - improve docs: add information that it checks for the existence of a module /!\ file /!\ name which is not the same as a module - improve tests: - proper cleanup of a golem - check failure/error if not inside a R-package (in the above sense) * tests: test create_if_need() function non-interactively - must produce error when called non-interactively for argyment type="file" * refactor: add clearer logic for create_if_needed() and test interactively 'no' - allow mocking of user interaction with "no"-response for testing purposes - requires encapsulation of yesno() to a separate function - restyle the file with grkstyler - add a test that mimicks user interation with "no" response of the yesno()-function => improve test coverage to 60.09 % * tests: add test for user interactive behavior "yes" whereby improving coverage to 62.62 % * refactor: - shallow-test encapsulated function ask_golem_creation_file() - remove redundant check_file_exist() which is never used inside `{golem}`! - improve formatting of some comments inside create_if_needed() => improve test coverage to 67.79% * tests: increaset R/utils.R coverage to 88.98% - snapshot tests save the output of cli-messages and check options - if cli:: package changes we see a different output and the snapshots will inform about this - additionally check do_if_unquiet() feature of these message functions --- R/utils.R | 103 ++++++++-------- tests/testthat/_snaps/utils.md | 30 +++++ tests/testthat/test-utils.R | 216 +++++++++++++++++++++++++++++++-- 3 files changed, 286 insertions(+), 63 deletions(-) create mode 100644 tests/testthat/_snaps/utils.md diff --git a/R/utils.R b/R/utils.R index 279d7a92..31edfb6f 100644 --- a/R/utils.R +++ b/R/utils.R @@ -2,7 +2,7 @@ golem_sys <- function( ..., lib.loc = NULL, mustWork = FALSE -) { + ) { system.file( ..., package = "golem", @@ -17,7 +17,7 @@ create_if_needed <- function( path, type = c("file", "directory"), content = NULL -) { + ) { type <- match.arg(type) # Check if file or dir already exist @@ -26,28 +26,20 @@ create_if_needed <- function( } else if (type == "directory") { dont_exist <- Negate(fs_dir_exists)(path) } - # If it doesn't exist, ask if we are allowed - # to create it + # If it doesn't exist, ask if we are allowed to create it if (dont_exist) { - if (interactive()) { - ask <- yesno( - sprintf( - "The %s %s doesn't exist, create?", - basename(path), - type - ) - ) + if (rlang::is_interactive()) { + ask <- ask_golem_creation_file(path, type) # Return early if the user doesn't allow if (!ask) { return(FALSE) - } else { - # Create the file - if (type == "file") { - fs_file_create(path) - write(content, path, append = TRUE) - } else if (type == "directory") { - fs_dir_create(path, recurse = TRUE) - } + } + # Create the file + if (type == "file") { + fs_file_create(path) + write(content, path, append = TRUE) + } else if (type == "directory") { + fs_dir_create(path, recurse = TRUE) } } else { stop( @@ -59,22 +51,17 @@ create_if_needed <- function( ) } } - - # TRUE means that file exists (either - # created or already there) + # TRUE means that file exists (either created or already there) return(TRUE) } - -check_file_exist <- function(file) { - res <- TRUE - if (fs_file_exists(file)) { - if (interactive()) { - res <- yesno("This file already exists, override?") - } else { - res <- TRUE - } - } - return(res) +ask_golem_creation_file <- function(path, type) { + yesno( + sprintf( + "The %s %s doesn't exist, create?", + basename(path), + type + ) + ) } # internal @@ -82,7 +69,7 @@ replace_word <- function( file, pattern, replace -) { + ) { suppressWarnings(tx <- readLines(file)) tx2 <- gsub( pattern = pattern, @@ -170,7 +157,7 @@ cat_start_download <- function() { cat_downloaded <- function( where, file = "File" -) { + ) { cat_green_tick( sprintf( "%s downloaded at %s", @@ -190,7 +177,7 @@ cat_start_copy <- function() { cat_copied <- function( where, file = "File" -) { + ) { cat_green_tick( sprintf( "%s copied to %s", @@ -203,7 +190,7 @@ cat_copied <- function( cat_created <- function( where, file = "File" -) { + ) { cat_green_tick( sprintf( "%s created at %s", @@ -224,7 +211,7 @@ cat_automatically_linked <- function() { open_or_go_to <- function( where, open_file -) { + ) { if ( open_file ) { @@ -250,7 +237,7 @@ after_creation_message_js <- function( pkg, dir, name -) { + ) { if ( desc_exist(pkg) ) { @@ -273,7 +260,7 @@ after_creation_message_css <- function( pkg, dir, name -) { + ) { if ( desc_exist(pkg) ) { @@ -296,7 +283,7 @@ after_creation_message_sass <- function( pkg, dir, name -) { + ) { if ( desc_exist(pkg) ) { @@ -316,7 +303,7 @@ after_creation_message_html_template <- function( pkg, dir, name -) { + ) { do_if_unquiet({ cli_cat_line("") cli_cat_line("To use this html file as a template, add the following code in your UI:") @@ -337,7 +324,7 @@ file_created_dance <- function( open_file, open_or_go_to = TRUE, catfun = cat_created -) { + ) { catfun(where) fun(pkg, dir, name) @@ -355,7 +342,7 @@ file_created_dance <- function( file_already_there_dance <- function( where, open_file -) { + ) { cat_green_tick("File already exists.") open_or_go_to( where = where, @@ -394,9 +381,9 @@ yesno <- function(...) { # Checking that a package is installed check_is_installed <- function( - pak, - ... -) { + pak, + ... + ) { if ( !requireNamespace(pak, ..., quietly = TRUE) ) { @@ -412,9 +399,9 @@ check_is_installed <- function( } required_version <- function( - pak, - version -) { + pak, + version + ) { if ( utils::packageVersion(pak) < version ) { @@ -464,14 +451,20 @@ add_sass_code <- function(where, dir, name) { } } -#' Check if a module already exists +#' Check if a module (`R`-file) already exists #' -#' Assumes it is called at the root of a golem project. +#' Should be called at the root of a `{golem}` project; but an error is thrown +#' only if one is not inside an R package (as the checks of `golem:::is_golem()` +#' are rather strict, specifically only the presence of a "R/" directory is +#' checked for the moment). #' -#' @param module A character string. The name of a potentially existing module -#' @return Boolean. Does the module exist or not ? +#' @param module a character string giving the name of a potentially existing +#' module `R`-file +#' @return boolean; `TRUE` if the module (`R`-file) exists and `FALSE` else #' @noRd is_existing_module <- function(module) { + stopifnot(`Cannot be called when not inside a R-package` = dir.exists("R")) + # stopifnot(`Cannot be called when not inside a golem-project` = is.golem()) existing_module_files <- list.files("R/", pattern = "^mod_") existing_module_names <- sub( "^mod_([[:alnum:]_]+)\\.R$", diff --git a/tests/testthat/_snaps/utils.md b/tests/testthat/_snaps/utils.md new file mode 100644 index 00000000..4ea5a33b --- /dev/null +++ b/tests/testthat/_snaps/utils.md @@ -0,0 +1,30 @@ +# simple cat-messaging functions keep output style when printing + + > Rebuild Berlin from scratch. + +--- + + + +--- + + + +--- + + + Initiating file download + +--- + + + Copying file + +--- + + + +--- + + + diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index b62ea043..57f30ae6 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -1,16 +1,216 @@ +test_that("is_existing_module() properly detects modules if they are present", { + path_dummy_golem <- tempfile(pattern = "dummygolem") + create_golem( + path = path_dummy_golem, + open = FALSE + ) + dummy_module_files <- c("mod_main.R", "mod_left_pane.R", "mod_pouet_pouet.R") + file.create(file.path(path_dummy_golem, "R", dummy_module_files)) -dummy_golem <- tempfile(pattern = "test_is_existing_modules") -dummy_golem_R <- file.path(dummy_golem, "R/") -dir.create(dummy_golem_R, recursive = TRUE) -dummy_module_files <- c("mod_main.R", "mod_left_pane.R", "mod_pouet_pouet.R") -file.create(file.path(dummy_golem_R, dummy_module_files)) - -withr::with_dir(dummy_golem, { - test_that("existing modules are properly detected", { + withr::with_dir(path_dummy_golem, { expect_false(is_existing_module("foo")) expect_true(is_existing_module("left_pane")) expect_true(is_existing_module("main")) expect_true(is_existing_module("pouet_pouet")) expect_false(is_existing_module("plif_plif")) }) + + # Cleanup + unlink(path_dummy_golem, TRUE, TRUE, TRUE) +}) + +test_that("is_existing_module() fails outside an R package", { + path_dummy_golem <- tempfile(pattern = "dummygolem") + create_golem( + path = path_dummy_golem, + open = FALSE + ) + + withr::with_dir(path_dummy_golem, { + dummy_module_files <- c("main.R", "left_pane.R", "pouet_pouet.R") + unlink("R/", TRUE, TRUE) + dir.create("RR/") + file.create(file.path(path_dummy_golem, "RR", dummy_module_files)) + expect_error( + is_existing_module("foo"), + "Cannot be called when not inside a R-package" + ) + expect_error( + is_existing_module("left_pane"), + "Cannot be called when not inside a R-package" + ) + expect_error( + is_existing_module("main"), + "Cannot be called when not inside a R-package" + ) + expect_error( + is_existing_module("pouet_pouet"), + "Cannot be called when not inside a R-package" + ) + expect_error( + is_existing_module("plif_plif"), + "Cannot be called when not inside a R-package" + ) + }) + + # Cleanup + unlink(path_dummy_golem, TRUE, TRUE, TRUE) +}) + +test_that("file creation utils fail non-interactively", { + path_dummy_golem <- tempfile(pattern = "dummygolem") + create_golem( + path = path_dummy_golem, + open = FALSE + ) + + withr::with_dir(path_dummy_golem, { + tmp_test_file_path <- file.path(getwd(), "R", "tmp_file.R") + expect_false(file.exists(tmp_test_file_path)) + expect_error( + rlang::with_interactive( + { + create_if_needed( + tmp_test_file_path, + type = "file", + content = "some text" + ) + }, + value = FALSE + ), + paste0("The tmp_file.R file doesn't exist.") + ) + expect_false(file.exists(tmp_test_file_path)) + }) + + # Cleanup + unlink(path_dummy_golem, TRUE, TRUE, TRUE) +}) + +test_that("file creation utils work interactively with user mimick 'no'", { + path_dummy_golem <- tempfile(pattern = "dummygolem") + create_golem( + path = path_dummy_golem, + open = FALSE + ) + + withr::with_dir(path_dummy_golem, { + tmp_test_file_path <- file.path(getwd(), "R", "tmp_file.R") + expect_false(file.exists(tmp_test_file_path)) + mockery::stub( + where = create_if_needed, + what = "ask_golem_creation_file", + how = FALSE + ) + expect_false( + rlang::with_interactive( + { + create_if_needed( + tmp_test_file_path, + type = "file", + content = "some text" + ) + }, + value = TRUE + ) + ) + expect_false(file.exists(tmp_test_file_path)) + }) + + # Cleanup + unlink(path_dummy_golem, TRUE, TRUE, TRUE) +}) + +test_that("file creation utils work interactively with user mimick 'yes'", { + path_dummy_golem <- tempfile(pattern = "dummygolem") + create_golem( + path = path_dummy_golem, + open = FALSE + ) + + withr::with_dir(path_dummy_golem, { + # 0. Define paths for tmp-file and tmp-dir to check existence for + tmp_test_file_path <- file.path(getwd(), "R", "tmp_file.R") + tmp_test_dir_path <- file.path(getwd(), "R", "tmp_dir") + # I. Expect that they do not exist + expect_false(file.exists(tmp_test_file_path)) + expect_false(dir.exists(tmp_test_dir_path)) + # II. replace user interaction from ask_golem_creation_file with TRUE + mockery::stub( + where = create_if_needed, + what = "ask_golem_creation_file", + how = TRUE + ) + # III. test that file creation works AS-IF the user says "yes"/TRUE + # III.A function must pass with return value TRUE + expect_true( + rlang::with_interactive( + { + create_if_needed( + tmp_test_file_path, + type = "file", + content = "some text" + ) + }, + value = TRUE + ) + ) + # III.B a file should be created + expect_true(file.exists(tmp_test_file_path)) + # III.C file content should match + check_content <- readLines(tmp_test_file_path) + expect_identical(check_content, "some text") + # IV. test that dir creation works AS-IF the user says "yes"/TRUE + # IV.A function must pass with return value TRUE + expect_true( + rlang::with_interactive( + { + create_if_needed( + tmp_test_dir_path, + type = "directory", + content = NULL + ) + }, + value = TRUE + ) + ) + # IV.B a dir should be created + expect_true(dir.exists(tmp_test_dir_path)) + }) + + # Cleanup + unlink(path_dummy_golem, TRUE, TRUE, TRUE) +}) + +test_that( + "ask_golem_creation_file() fails in non-interactive mode", + { + # Shallow testing to improve code-coverage + expect_error(ask_golem_creation_file("test/path", "some_type")) + } +) + +test_that("simple cat-messaging functions keep output style when printing", { + # This is achieved by snapping their output. If {cli} changes output style we + # will notice it via a different snapshot here. and can adjust accordingly. + withr::with_options( + list(golem.quiet = FALSE), + expect_snapshot_output(cat_info("Rebuild Berlin from scratch.")) + ) + expect_snapshot_output(cat_exists("build/city/Berlin")) + expect_snapshot_output(cat_dir_necessary()) + withr::with_options( + list(golem.quiet = FALSE), + expect_snapshot_output(cat_start_download()) + ) + withr::with_options( + list(golem.quiet = FALSE), + expect_snapshot_output(cat_start_copy()) + ) + expect_snapshot_output(cat_downloaded("a place with no wifi.")) + expect_snapshot_output(cat_copied("inside a black hole.")) + withr::with_options( + list(golem.quiet = FALSE), + after_creation_message_html_template(name = "NAME-OF-HTML-FILE") + ) })