Merge branch 'main' into tidy-dependencies

.Rbuildignore

@@ -18,6 +18,6 @@
 ^man-roxygen$
 ^LICENSE\.md$
 ^MAINTENANCE\.md$
-^\.lintr$
+^\.lintr\.R$
 ^notes$
 ^CRAN-SUBMISSION$

.github/workflows/pkgdown.yaml

@@ -19,6 +19,8 @@ jobs:
       group: pkgdown-${{ github.event_name != 'pull_request' || github.run_id }}
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+    permissions:
+      contents: write
     steps:
       - uses: actions/checkout@v3
 

.github/workflows/test-coverage.yaml

@@ -31,7 +31,7 @@ jobs:
           covr::codecov(
             quiet = FALSE,
             clean = FALSE,
-            install_path = file.path(Sys.getenv("RUNNER_TEMP"), "package")
+            install_path = file.path(normalizePath(Sys.getenv("RUNNER_TEMP"), winslash = "/"), "package")
           )
         shell: Rscript {0}
 

.lintr → .lintr.R

@@ -1,4 +1,5 @@
-linters: funs <- c(
+linters <- list(lintr::undesirable_function_linter(
+  fun = c(
     # Base messaging
     "message" = "use cli::cli_inform()",
     "warning" = "use cli::cli_warn()",
@@ -12,9 +13,6 @@ linters: funs <- c(
     "cli_alert_info" = "use cli::cli_inform()",
     "cli_alert_success" = "use cli::cli_inform()",
     "cli_alert_warning" = "use cli::cli_inform()"
-  )
-  list(lintr::undesirable_function_linter(
-    fun = funs,
-    symbol_is_undesirable = FALSE
-  ))
-encoding: "UTF-8"
+  ),
+  symbol_is_undesirable = FALSE
+))

DESCRIPTION

@@ -7,14 +7,14 @@ Authors@R: c(
     person("Winston", "Chang", role = "aut"),
     person("Jennifer", "Bryan", , "jenny@posit.co", role = c("aut", "cre"),
            comment = c(ORCID = "0000-0002-6983-2759")),
-    person(given = "Posit Software, PBC", role = c("cph", "fnd"))
+    person("Posit Software, PBC", role = c("cph", "fnd"))
   )
 Description: Collection of package development tools.
 License: MIT + file LICENSE
 URL: https://devtools.r-lib.org/, https://github.com/r-lib/devtools
 BugReports: https://github.com/r-lib/devtools/issues
 Depends: 
-    R (>= 3.0.2),
+    R (>= 3.6),
     usethis (>= 2.1.6)
 Imports: 
     callr (>= 3.7.3),
@@ -39,7 +39,7 @@ Imports:
     rversions (>= 2.1.2),
     sessioninfo (>= 1.2.2),
     stats,
-    testthat (>= 3.1.6),
+    testthat (>= 3.1.10.9000),
     tools,
     urlchecker (>= 1.0.1),
     utils,
@@ -63,9 +63,11 @@ Suggests:
     spelling (>= 2.2)
 VignetteBuilder: 
     knitr
+Remotes: 
+    r-lib/testthat
 Config/Needs/website: tidyverse/tidytemplate
+Config/testthat/edition: 3
 Encoding: UTF-8
 Language: en-US
 Roxygen: list(markdown = TRUE)
 RoxygenNote: 7.2.3
-Config/testthat/edition: 3

LICENSE

@@ -1,2 +1,2 @@
-YEAR: 2021
+YEAR: 2023
 COPYRIGHT HOLDER: devtools authors

LICENSE.md

@@ -1,6 +1,6 @@
 # MIT License
 
-Copyright (c) 2021 devtools authors
+Copyright (c) 2023 devtools authors
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

MAINTENANCE.md

@@ -11,3 +11,17 @@ I don't know of any major outstanding issues in devtools itself.
 The package development cheatsheet likely needs a major overhaul to account for much more functionality in usethis and the current state of devtools. https://github.com/r-lib/devtools/issues/2107
 
 Should devtools be converted to use pak for installation, or should the installation commands be deprecated in devtools and users suggested to use pak directly?
+
+## R CMD check notes
+
+devtools has some vintage tests around `.Rnw` vignettes.
+It's not clear if it makes sense to keep these, but I (jennybc) am not rushing to remove these tests.
+Since I inherited the maintainership of devtools, I have not pursued why 2 of these tests fail locally (they are skipped on GHA).
+However, today, I did sort it out and want to record what I did.
+In case it comes up in the future, this caused the necessary LaTeX package to be installed:
+
+``` r
+tinytex::parse_install(
+  text = "! LaTeX Error: File `grfext.sty' not found."
+)
+```

NAMESPACE

@@ -87,15 +87,13 @@ importFrom(ellipsis,check_dots_used)
 importFrom(glue,glue)
 importFrom(lifecycle,deprecated)
 importFrom(memoise,memoise)
-importFrom(miniUI,miniPage)
 importFrom(pkgbuild,clean_dll)
 importFrom(pkgbuild,find_rtools)
 importFrom(pkgbuild,has_devel)
 importFrom(pkgbuild,with_debug)
 importFrom(pkgload,check_dep_version)
 importFrom(pkgload,parse_deps)
 importFrom(pkgload,unload)
-importFrom(profvis,profvis)
 importFrom(remotes,dev_package_deps)
 importFrom(remotes,github_pull)
 importFrom(remotes,github_release)
@@ -114,7 +112,6 @@ importFrom(remotes,update_packages)
 importFrom(sessioninfo,package_info)
 importFrom(sessioninfo,session_info)
 importFrom(stats,update)
-importFrom(urlchecker,url_check)
 importFrom(usethis,ui_code)
 importFrom(usethis,ui_done)
 importFrom(usethis,ui_field)

NEWS.md

@@ -1,5 +1,10 @@
 # devtools (development version)
 
+* `test_coverage()` now works if the package has not been installed.
+
+* `test_coverage_active_file()` now reports if any tests failed and does
+  a better job of executing snapshot comparisons.
+
 # devtools 2.4.5
 
 * `check(cleanup =)` was deprecated in devtools v1.11.0 (2016-04-12) and was
@@ -907,7 +912,7 @@ There were a handful of smaller fixes:
   (#995, @kevinushey)
 
 * `check(cran = TRUE)` adds `--run-donttest` since you do need to test
-  code in `\dontest()` for CRAN submission (#1002).
+  code in `\donttest()` for CRAN submission (#1002).
 
 ## Package installation
 
@@ -939,7 +944,7 @@ There were a handful of smaller fixes:
 
 * `check_man()` replaces `check_doc()` (since most other functions are
   named after the corresponding directory). `check_doc()` will hang around
-  as an alias for the forseeable future (#958).
+  as an alias for the foreseeable future (#958).
 
 * `create()` produces a dummy namespace will fake comment so roxygen2 will 
   overwrite silently (#1016).

R/active.R

@@ -15,8 +15,10 @@ find_test_file <- function(path, call = parent.frame()) {
     )
   }
 
+  pkg <- as.package(dirname(path))
+
   is_test <- type == "test"
-  path[!is_test] <- glue("tests/testthat/test-{name_source(path[!is_test])}.R")
+  path[!is_test] <- glue("{pkg$path}/tests/testthat/test-{name_source(path[!is_test])}.R")
   path <- unique(path[file_exists(path)])
 
   if (length(path) == 0) {

R/check.R

@@ -21,23 +21,19 @@
 #' Devtools does its best to set up an environment that combines best practices
 #' with how check works on CRAN. This includes:
 #'
-#' \itemize{
+#' * The standard environment variables set by devtools:
+#' [r_env_vars()]. Of particular note for package tests is the `NOT_CRAN` env
+#' var, which lets you know that your tests are running somewhere other than
+#' CRAN, and hence can take a reasonable amount of time.
 #'
-#'  \item The standard environment variables set by devtools:
-#'    [r_env_vars()]. Of particular note for package tests is the
-#'    `NOT_CRAN` env var which lets you know that your tests are not
-#'    running on CRAN, and hence can take a reasonable amount of time.
+#' * Debugging flags for the compiler, set by
+#' [`compiler_flags(FALSE)`][compiler_flags()].
 #'
-#'  \item Debugging flags for the compiler, set by
-#'    \code{\link{compiler_flags}(FALSE)}.
+#' * If `aspell` is found, `_R_CHECK_CRAN_INCOMING_USE_ASPELL_`
+#' is set to `TRUE`. If no spell checker is installed, a warning is issued.
 #'
-#'  \item If `aspell` is found `_R_CHECK_CRAN_INCOMING_USE_ASPELL_`
-#'   is set to `TRUE`. If no spell checker is installed, a warning is
-#'   issued.)
-#'
-#'  \item env vars set by arguments `incoming`, `remote` and
-#'    `force_suggests`
-#' }
+#' * Environment variables, controlled by arguments `incoming`, `remote` and
+#' `force_suggests`.
 #'
 #' @return An object containing errors, warnings, notes, and more.
 #' @template devtools
@@ -175,7 +171,14 @@ can_document <- function(pkg) {
 #' @param manual If `FALSE`, don't build and check manual (`--no-manual`).
 #' @param env_vars Environment variables set during `R CMD check`
 #' @param quiet if `TRUE` suppresses output from this function.
-#' @inheritParams rcmdcheck::rcmdcheck
+#' @param error_on Whether to throw an error on `R CMD check` failures. Note
+#'   that the check is always completed (unless a timeout happens), and the
+#'   error is only thrown after completion.
+#'
+#'   `error_on` is passed through to [rcmdcheck::rcmdcheck()], which is the
+#'   definitive source for what the different values mean. If not specified by
+#'   the user, both `check()` and `check_built()` default to `error_on =
+#'   "never"` in interactive use and `"warning"` in a non-interactive setting.
 check_built <- function(path = NULL, cran = TRUE,
                         remote = FALSE, incoming = remote, force_suggests = FALSE,
                         run_dont_test = FALSE, manual = FALSE, args = "--timings",

R/devtools-package.R

@@ -13,8 +13,12 @@
 #' @import rlang
 #' @importFrom glue glue
 #' @importFrom lifecycle deprecated
-#' @importFrom miniUI miniPage
-#' @importFrom profvis profvis
-#' @importFrom urlchecker url_check
 ## usethis namespace: end
 NULL
+
+# https://r-pkgs.org/dependencies-in-practice.html#how-to-not-use-a-package-in-imports
+ignore_unused_imports <- function() {
+  miniUI::miniPage
+  profvis::profvis
+  urlchecker::url_check
+}

R/release.R

@@ -4,12 +4,10 @@
 #'
 #' The package release process will:
 #'
-#' \itemize{
-#'   \item Confirm that the package passes `R CMD check` on relevant platforms
-#'   \item Confirm that important files are up-to-date
-#'   \item Build the package
-#'   \item Submit the package to CRAN, using comments in "cran-comments.md"
-#' }
+#'   * Confirm that the package passes `R CMD check` on relevant platforms
+#'   * Confirm that important files are up-to-date
+#'   * Build the package
+#'   * Submit the package to CRAN, using comments in "cran-comments.md"
 #'
 #' You can add arbitrary extra questions by defining an (un-exported) function
 #' called `release_questions()` that returns a character vector
@@ -202,14 +200,26 @@ cran_comments <- function(pkg = ".", call = parent.frame()) {
 
 cran_submission_url <- "https://xmpalantir.wu.ac.at/cransubmit/index2.php"
 
-#' Submit a package to CRAN.
+#' Submit a package to CRAN
 #'
-#' This uses the new CRAN web-form submission process. After submission, you
-#' will receive an email asking you to confirm submission - this is used
-#' to check that the package is submitted by the maintainer.
+#' @description
+
+#' This submits your package to CRAN using the web-form submission process.
+#' After submission, you will receive an email asking you to confirm submission
+#' - this is used to check that the package is submitted by the maintainer.
+#'
+#' You may prefer to use `submit_cran()` indirectly, by calling [release()]
+#' instead. `release()` performs many checks verifying that your package is
+#' indeed ready for CRAN, before eventually asking for your confirmation that
+#' you'd like to submit it to CRAN (which it does by calling `submit_cran()`).
 #'
-#' It's recommended that you use [release()] rather than this
-#' function as it performs more checks prior to submission.
+#' Whether to use `release()` or `submit_cran()` depends on the rest of your
+#' development process. If you want to be super cautious, use `release()`, even
+#' though it may be redundant with other checks you have performed. On the other
+#' hand, if you have many other checks in place (such as automated checks via
+#' GitHub Actions and the task list generated by
+#' [usethis::use_release_issue()]), it makes sense to use `submit_cran()`
+#' directly.
 #'
 #' @template devtools
 #' @inheritParams release

R/test.R

@@ -38,7 +38,7 @@ test <- function(pkg = ".", filter = NULL, stop_on_failure = FALSE, export_all =
   cli::cli_inform(c(i = "Testing {.pkg {pkg$package}}"))
   withr::local_envvar(r_env_vars())
 
-  load_package <- load_package_if_needed(pkg)
+  load_package <- load_package_for_testing(pkg)
   testthat::test_local(
     pkg$path,
     filter = filter,
@@ -67,7 +67,7 @@ test_active_file <- function(file = find_active_file(), ...) {
     rstudioapi::executeCommand("activateConsole", quiet = TRUE)
   }
 
-  load_package <- load_package_if_needed(pkg)
+  load_package <- load_package_for_testing(pkg)
   testthat::test_file(
     test_files,
     package = pkg$package,
@@ -76,11 +76,11 @@ test_active_file <- function(file = find_active_file(), ...) {
   )
 }
 
-load_package_if_needed <- function(pkg) {
+load_package_for_testing <- function(pkg) {
   if (pkg$package == "testthat") {
     # Must load testthat outside of testthat so tests are run with
     # dev testthat
-    load_all(pkg$path, quiet = TRUE)
+    load_all(pkg$path, quiet = TRUE, helpers = FALSE)
     "none"
   } else {
     "source"
@@ -100,7 +100,6 @@ test_coverage <- function(pkg = ".", show_report = interactive(), ...) {
   check_dots_used(action = getOption("devtools.ellipsis_action", rlang::warn))
 
   withr::local_envvar(r_env_vars())
-  testthat::local_test_directory(pkg$path, pkg$package)
   coverage <- covr::package_coverage(pkg$path, ...)
 
   if (isTRUE(show_report)) {
@@ -119,28 +118,42 @@ test_coverage_file <- function(file = find_active_file(), ...) {
 
 #' @rdname test
 #' @export
-test_coverage_active_file <- function(file = find_active_file(), filter = TRUE, show_report = interactive(), export_all = TRUE, ...) {
+test_coverage_active_file <- function(file = find_active_file(),
+                                      filter = TRUE,
+                                      show_report = interactive(),
+                                      export_all = TRUE,
+                                      ...) {
   check_installed(c("covr", "DT"))
-
-  save_all()
-  test_files <- find_test_file(file)
-  pkg <- as.package(path_dir(file)[[1]])
-
   check_dots_used(action = getOption("devtools.ellipsis_action", rlang::warn))
 
-  withr::local_envvar(r_env_vars())
-  testthat::local_test_directory(pkg$path, pkg$package)
-  reporter <- testthat::local_snapshotter()
-  reporter$start_file(file, "test")
+  test_file <- find_test_file(file)
+  test_dir <- path_dir(test_file)
+  pkg <- as.package(test_dir)
 
   env <- load_all(pkg$path, quiet = TRUE, export_all = export_all)$env
+  # this always ends up using the package DESCRIPTION, which will refer
+  # to the source package because of the load_all() above
+  testthat::local_test_directory(test_dir, pkg$package)
+
+  # To correctly simulate test_file() we need to set up both a temporary
+  # snapshotter (with correct directory specification) for snapshot comparisons
+  # and a stop reporter to inform the user about test failures
+  snap_reporter <- testthat::local_snapshotter(file.path(test_dir, "_snaps"))
+  snap_reporter$start_file(basename(test_file))
+  reporter <- testthat::MultiReporter$new(reporters = list(
+    testthat::StopReporter$new(praise = FALSE),
+    snap_reporter
+  ))
+
+  withr::local_envvar(r_env_vars())
   testthat::with_reporter(reporter, {
-    coverage <- covr::environment_coverage(env, test_files, ...)
+    coverage <- covr::environment_coverage(env, test_file, ...)
+    reporter$end_file() # needed to write new snapshots
   })
 
   if (isTRUE(filter)) {
     coverage_name <- name_source(covr::display_name(coverage))
-    local_name <- name_test(file)
+    local_name <- name_test(test_file)
     coverage <- coverage[coverage_name %in% local_name]
   }