From 66db96d017194f6af97e4c326dc6580607b23b1b Mon Sep 17 00:00:00 2001 From: Tymoteusz Makowski Date: Thu, 14 Sep 2023 20:35:03 +0200 Subject: [PATCH 1/3] docs: update hooks list --- vignettes/hook-order.Rmd | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/vignettes/hook-order.Rmd b/vignettes/hook-order.Rmd index 5e897969d..47c86ca81 100644 --- a/vignettes/hook-order.Rmd +++ b/vignettes/hook-order.Rmd @@ -21,21 +21,23 @@ Read only hooks should generally run only after write hooks. - Only add a dependency once, i.e. if styler must run before roxygen, add the requirement to styler or roxygen, not both. This makes keeping track easier. - The hooks must appear in an order that meets all constraints, not just randomly order constraints. -## Hooks with dependencies: +## Hooks with dependencies -**Read and write** +### Read and write -- styler: should run before roxygen because of caching. Caches. -- roxygen. Caches. -- codemeta: must be before tidy description. -- use-tidy-description. +- style-files - caches; should run before roxygenize because of the caching +- roxygenize - caches +- codemeta-description-updated - must be before use-tidy-description +- use-tidy-description +- spell-check - updates `inst/WORDLIST`; should run after roxygenize -**Just read:** +### Read only -- spell check; run after roxygen -- lintr: should run after styler. -- readme-rmd-rendered: Must run after styler. +- lintr - should run after styler +- readme-rmd-rendered - must run after styler - parsable-R - no-browser-statement - no-print-statement -- deps in desc. +- no-debug-statement +- deps-in-desc +- pkgdown From 8962b2071a693e79f0796ac3a41016a4f50b8b33 Mon Sep 17 00:00:00 2001 From: Tymoteusz Makowski Date: Fri, 10 Nov 2023 12:54:32 +0100 Subject: [PATCH 2/3] feat: add read_only argument to run_test --- R/testing.R | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/R/testing.R b/R/testing.R index 2a3debb27..b5db5e23f 100644 --- a/R/testing.R +++ b/R/testing.R @@ -43,7 +43,8 @@ run_test <- function(hook_name, artifacts = NULL, file_transformer = function(files) files, env = character(), - expect_success = is.null(std_err)) { + expect_success = is.null(std_err), + read_only = FALSE) { withr::local_envvar(list(R_PRECOMMIT_HOOK_ENV = "1")) path_executable <- fs::dir_ls(system.file( fs::path("hooks", "exported"), @@ -62,7 +63,8 @@ run_test <- function(hook_name, artifacts = ensure_named(artifacts), file_transformer = file_transformer, env = env, - expect_success = expect_success + expect_success = expect_success, + read_only = read_only ) } @@ -93,7 +95,8 @@ run_test_impl <- function(path_executable, artifacts, file_transformer, env, - expect_success) { + expect_success, + read_only) { # ensure cannonical /private/var/... not /var/... on macOS tempdir <- fs::path(normalizePath((fs::dir_create(fs::file_temp())))) copy_artifacts(artifacts, tempdir) @@ -128,6 +131,13 @@ run_test_impl <- function(path_executable, std_out, exit_status ) + if (isTRUE(read_only) && !is.null(artifacts)) { + purrr::iwalk(artifacts, function(reference_path, temp_path) { + artifact_before_hook <- readLines(testthat::test_path(reference_path)) + artifact_after_hook <- readLines(fs::path_join(c(tempdir, temp_path))) + testthat::expect_equal(artifact_before_hook, artifact_after_hook) + }) + } } From 9a993194ee1d79aee60df059ec269eaf86dfb3f7 Mon Sep 17 00:00:00 2001 From: Tymoteusz Makowski Date: Fri, 10 Nov 2023 13:42:56 +0100 Subject: [PATCH 3/3] docs: add read_only docs --- R/testing.R | 2 ++ man/run_test.Rd | 6 +++++- man/run_test_impl.Rd | 6 +++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/R/testing.R b/R/testing.R index b5db5e23f..2a9cbc9f0 100644 --- a/R/testing.R +++ b/R/testing.R @@ -86,6 +86,8 @@ run_test <- function(hook_name, #' @param expect_success Whether or not an exit code 0 is expected. This can #' be derived from `std_err`, but sometimes, non-empty stderr does not mean #' error, but just a message. +#' @param read_only If `TRUE` and `artifacts` are not `NULL`, then assert that hook +#' did not modify the artifacts. #' @keywords internal run_test_impl <- function(path_executable, path_candidate, diff --git a/man/run_test.Rd b/man/run_test.Rd index fbc8f4388..f6aa2c84a 100644 --- a/man/run_test.Rd +++ b/man/run_test.Rd @@ -14,7 +14,8 @@ run_test( artifacts = NULL, file_transformer = function(files) files, env = character(), - expect_success = is.null(std_err) + expect_success = is.null(std_err), + read_only = FALSE ) } \arguments{ @@ -55,6 +56,9 @@ made.} \item{expect_success}{Whether or not an exit code 0 is expected. This can be derived from \code{std_err}, but sometimes, non-empty stderr does not mean error, but just a message.} + +\item{read_only}{If \code{TRUE} and \code{artifacts} are not \code{NULL}, then assert that hook +did not modify the artifacts.} } \description{ Tests for the executables used as pre-commit hooks via \code{entrypoint} in diff --git a/man/run_test_impl.Rd b/man/run_test_impl.Rd index 534babd0d..e060309b1 100644 --- a/man/run_test_impl.Rd +++ b/man/run_test_impl.Rd @@ -13,7 +13,8 @@ run_test_impl( artifacts, file_transformer, env, - expect_success + expect_success, + read_only ) } \arguments{ @@ -40,6 +41,9 @@ temporary location and the value is the source of the file.} \item{expect_success}{Whether or not an exit code 0 is expected. This can be derived from \code{std_err}, but sometimes, non-empty stderr does not mean error, but just a message.} + +\item{read_only}{If \code{TRUE} and \code{artifacts} are not \code{NULL}, then assert that hook +did not modify the artifacts.} } \description{ Implement a test run