From 4ba4a290c50db26c0d6d02406fd72d6060a24e51 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sun, 10 Jul 2022 18:16:58 +0100 Subject: [PATCH 1/7] support argument root for deps in desc --- inst/hooks/exported/deps-in-desc.R | 7 ++++++- tests/testthat/test-hooks.R | 20 ++++++++++++++++++++ vignettes/available-hooks.Rmd | 26 ++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/inst/hooks/exported/deps-in-desc.R b/inst/hooks/exported/deps-in-desc.R index c1c13fdc3..480598929 100755 --- a/inst/hooks/exported/deps-in-desc.R +++ b/inst/hooks/exported/deps-in-desc.R @@ -2,10 +2,12 @@ "Ensure all dependencies of the form pkg::fun are in DESCRIPTION Usage: - deps-in-desc [--allow_private_imports] ... + deps-in-desc [--allow_private_imports] [--root=]... Options: --allow_private_imports Whether or not to allow the use of ::: on imported functions. + --root= Path relative to the git root that contains the R package root [default: .]. + " -> doc pre_installed <- c( "base", "boot", "class", "cluster", "codetools", "compiler", @@ -16,6 +18,8 @@ pre_installed <- c( ) arguments <- docopt::docopt(doc) +arguments$files <- normalizePath(arguments$files) # because working directory changes to root +setwd(normalizePath(arguments$root)) deps_in_desc <- function(file, arguments) { if (basename(file) == "README.Rmd") { # is .Rbuildignored, dependencies irrelevant @@ -70,6 +74,7 @@ deps_in_desc <- function(file, arguments) { } out } + out <- lapply(arguments$files, deps_in_desc, arguments = arguments) if (!all(unlist(out))) { stop("Dependency check failed", call. = FALSE) diff --git a/tests/testthat/test-hooks.R b/tests/testthat/test-hooks.R index fce703481..2d531cf5d 100644 --- a/tests/testthat/test-hooks.R +++ b/tests/testthat/test-hooks.R @@ -213,6 +213,26 @@ run_test("deps-in-desc", artifacts = c("DESCRIPTION" = test_path("in/DESCRIPTION")) ) +# in sub directory with wrong root +run_test("deps-in-desc", + suffix = "-fail.R", std_err = "contains a file", + artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) +) + +# in sub directory with correct root +run_test("deps-in-desc", + cmd_args = "--root=rpkg", + suffix = "-fail.R", std_err = "Dependency check failed", + artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) +) + +# in sub directory with correct root +run_test("deps-in-desc", + cmd_args = "--root=rpkg", + suffix = "-success.R", std_err = NULL, + artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) +) + # with ::: run_test("deps-in-desc", "deps-in-desc-dot3", diff --git a/vignettes/available-hooks.Rmd b/vignettes/available-hooks.Rmd index 320909806..9af7246a4 100644 --- a/vignettes/available-hooks.Rmd +++ b/vignettes/available-hooks.Rmd @@ -226,14 +226,32 @@ roclets you specified in `DESCRIPTION`. ## `deps-in-desc` Checks if packages used with the `pkgname::fun()` syntax are listed in -your DESCRIPTION file. Note that `README.Rmd` is never checked. Flag -`allow_private_imports` lets the user specify that private imports into -the package namespace are tolerable, e.g. `somepkg:::x()`. Flag not set -by default, i.e. the hook will fail if such a call is found. +your DESCRIPTION file. Note that `README.Rmd` is never checked. +**Arguments** + +- Flag `allow_private_imports` lets the user specify that private + imports into the package namespace are tolerable, e.g. + `somepkg:::x()`. Flag not set by default, i.e. the hook will fail if + such a call is found. + +```{=html} + +``` id: deps-in-desc args: [--allow_private_imports] +- Argument `root` specifies the directory in the git repo that + contains the R package. Defaults to `.` since for most R package git + repos, the git and R package root coincide. Added in version + 0.3.2.9000. + +```{=html} + +``` + id: deps-in-desc + args: [--root=] + This hook does not modify the file `DESCRIPTION` because the user should decide for each package if it should go to `Imports:` or `Suggests:`, which can be done easily with `usethis::use_package()`. From 47b6df46e67dc14013aae462aae37b76b06d73cc Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 11 Jul 2022 10:37:33 +0100 Subject: [PATCH 2/7] fix visual --- vignettes/available-hooks.Rmd | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vignettes/available-hooks.Rmd b/vignettes/available-hooks.Rmd index 9af7246a4..37735f0f5 100644 --- a/vignettes/available-hooks.Rmd +++ b/vignettes/available-hooks.Rmd @@ -235,9 +235,8 @@ your DESCRIPTION file. Note that `README.Rmd` is never checked. `somepkg:::x()`. Flag not set by default, i.e. the hook will fail if such a call is found. -```{=html} -``` + id: deps-in-desc args: [--allow_private_imports] @@ -246,9 +245,8 @@ your DESCRIPTION file. Note that `README.Rmd` is never checked. repos, the git and R package root coincide. Added in version 0.3.2.9000. -```{=html} -``` + id: deps-in-desc args: [--root=] From 82d79fbd33f3290186e98b7f23a54103b7a52025 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 11 Jul 2022 10:38:26 +0100 Subject: [PATCH 3/7] fix spacing --- inst/hooks/exported/deps-in-desc.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/hooks/exported/deps-in-desc.R b/inst/hooks/exported/deps-in-desc.R index 480598929..3d0f500fd 100755 --- a/inst/hooks/exported/deps-in-desc.R +++ b/inst/hooks/exported/deps-in-desc.R @@ -2,7 +2,7 @@ "Ensure all dependencies of the form pkg::fun are in DESCRIPTION Usage: - deps-in-desc [--allow_private_imports] [--root=]... + deps-in-desc [--allow_private_imports] [--root=] ... Options: --allow_private_imports Whether or not to allow the use of ::: on imported functions. From a519609a9a8f1a49c2e532ece6bf41f9a06b9226 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 11 Jul 2022 10:45:04 +0100 Subject: [PATCH 4/7] file paths are now absolute in error messages too and `std_out` checks for exact match --- tests/testthat/test-hooks.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-hooks.R b/tests/testthat/test-hooks.R index 2d531cf5d..580b8beb0 100644 --- a/tests/testthat/test-hooks.R +++ b/tests/testthat/test-hooks.R @@ -257,7 +257,7 @@ run_test("deps-in-desc", run_test("deps-in-desc", "deps-in-desc", suffix = "-fail.Rmd", std_err = "Dependency check failed", - std_out = "in file `deps-in-desc-fail.Rmd`", + std_out = "deps-in-desc-fail.Rmd`: ttyzp", artifacts = c("DESCRIPTION" = test_path("in/DESCRIPTION")) ) From 1a426919b8d1c95b9c89f71a70a64eac541f6aa4 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 1 Aug 2022 10:17:30 +0200 Subject: [PATCH 5/7] ensure cannonical temp dir --- R/testing.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/testing.R b/R/testing.R index a0c2f3fa5..4e3295fbe 100644 --- a/R/testing.R +++ b/R/testing.R @@ -94,7 +94,8 @@ run_test_impl <- function(path_executable, file_transformer, env, expect_success) { - tempdir <- fs::dir_create(fs::file_temp()) + # ensure cannonical /private/var/... not /var/... on macOS + tempdir <- fs::path(normalizePath((fs::dir_create(fs::file_temp())))) copy_artifacts(artifacts, tempdir) # if name set use this, otherwise put in root path_candidate_temp <- fs::path(tempdir, names(path_candidate)) From 60da7415d079077d3587f59d4abc68ed314b610f Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 1 Aug 2022 10:18:05 +0200 Subject: [PATCH 6/7] ensure that source files are in root/rpkg, i.e. where the R package is, not one level above --- tests/testthat/test-hooks.R | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/testthat/test-hooks.R b/tests/testthat/test-hooks.R index 580b8beb0..de1fd9956 100644 --- a/tests/testthat/test-hooks.R +++ b/tests/testthat/test-hooks.R @@ -216,6 +216,9 @@ run_test("deps-in-desc", # in sub directory with wrong root run_test("deps-in-desc", suffix = "-fail.R", std_err = "contains a file", + file_transformer = function(files) { + fs::path_abs(fs::file_move(files, "rpkg")) + }, artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) ) @@ -223,6 +226,9 @@ run_test("deps-in-desc", run_test("deps-in-desc", cmd_args = "--root=rpkg", suffix = "-fail.R", std_err = "Dependency check failed", + file_transformer = function(files) { + fs::path_abs(fs::file_move(files, "rpkg")) + }, artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) ) @@ -230,6 +236,9 @@ run_test("deps-in-desc", run_test("deps-in-desc", cmd_args = "--root=rpkg", suffix = "-success.R", std_err = NULL, + file_transformer = function(files) { + fs::path_abs(fs::file_move(files, "rpkg")) + }, artifacts = c("rpkg/DESCRIPTION" = test_path("in/DESCRIPTION")) ) From 83914bd4acd916f70498ad3f33fb7ddfd0f5fe64 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Tue, 2 Aug 2022 14:34:29 +0200 Subject: [PATCH 7/7] avoid devtools dep this now has new system dep: https://github.com/r-lib/devtools/commit/575ae4e8d2cd445e89e3eed569cf97905bc024de --- .github/workflows/hook-tests.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/hook-tests.yaml b/.github/workflows/hook-tests.yaml index a27f37217..8560022b5 100644 --- a/.github/workflows/hook-tests.yaml +++ b/.github/workflows/hook-tests.yaml @@ -88,7 +88,7 @@ jobs: } # avoid build-time deps options(install.packages.compile.from.source = "never") - renv::install(c('testthat', 'devtools', 'git2r')) + renv::install(c('testthat', 'pkgload', 'git2r')) # some testing infrastructure we need is in R/testing.R renv::install(getwd()) shell: Rscript {0} @@ -101,7 +101,7 @@ jobs: shell: Rscript {0} - name: Test run: | - devtools::load_all() + pkgload::load_all() testthat::test_file( "tests/testthat/test-hooks.R", reporter = testthat::MultiReporter$new(list(