Skip to content

Commit

Permalink
Merge pull request #529 from TymekDev/run-test-update
Browse files Browse the repository at this point in the history
Add read_only flag to run_test
  • Loading branch information
lorenzwalthert authored Jan 7, 2024
2 parents 0a0f75c + 9a99319 commit 4f1a7e5
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 16 deletions.
18 changes: 15 additions & 3 deletions R/testing.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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
)
}

Expand All @@ -84,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,
Expand All @@ -93,7 +97,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)
Expand Down Expand Up @@ -128,6 +133,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)
})
}
}


Expand Down
6 changes: 5 additions & 1 deletion man/run_test.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion man/run_test_impl.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 13 additions & 11 deletions vignettes/hook-order.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 4f1a7e5

Please sign in to comment.