Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move withr from imports to suggests #291

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ Imports:
processx,
rlang (>= 1.1.1),
rprojroot,
utils,
withr (>= 2.4.3)
utils
Suggests:
bitops,
jsonlite,
Expand All @@ -40,7 +39,8 @@ Suggests:
remotes,
rstudioapi,
testthat (>= 3.2.1.1),
usethis
usethis,
withr
Config/Needs/website: tidyverse/tidytemplate, ggplot2
Config/testthat/edition: 3
Config/testthat/parallel: TRUE
Expand Down
24 changes: 11 additions & 13 deletions R/compilation-db.R
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,17 @@

# The `OBJECTS` variable should not depend on R variables, so we can inspect
# it in isolation
withr::with_dir(
src_path,
pkgbuild::with_build_tools(
out <- processx::run(
"make",
c(
"-f",
makevars,
"-f",
system.file("print-var.mk", package = "pkgload"),
"print-OBJECTS"
),
)
local_dir(src_path)
pkgbuild::with_build_tools(
out <- processx::run(
"make",
c(
"-f",
makevars,
"-f",
system.file("print-var.mk", package = "pkgload"),
"print-OBJECTS"
),

Check warning on line 88 in R/compilation-db.R

View check run for this annotation

Codecov / codecov/patch

R/compilation-db.R#L78-L88

Added lines #L78 - L88 were not covered by tests
)
)

Expand Down
3 changes: 2 additions & 1 deletion R/dev-help.R
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ topic_write_html <- function(x, path) {
}

topic_lines <- function(x, type = c("text", "html")) {
file <- withr::local_tempfile()
file <- tempfile()
defer(unlink(file))

switch(
arg_match(type),
Expand Down
2 changes: 1 addition & 1 deletion R/enc.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
read_lines_enc <- function(path, file_encoding = "UTF-8", n = -1L, ok = TRUE, skipNul = FALSE) {
con <- file(path, encoding = file_encoding)
on.exit(close(con), add = TRUE)
defer(close(con))

lines <- readLines(con, warn = FALSE, n = n, ok = ok, skipNul = skipNul)
Encoding(lines) <- "UTF-8"
Expand Down
13 changes: 8 additions & 5 deletions R/load-code.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ load_code <- function(path = ".", quiet = NULL) {
clear_cache()
unload(package)
}
on.exit(cleanup())
defer(cleanup())

withr_with_dir(path, source_many(paths, encoding, env))
local({
local_dir(path)
source_many(paths, encoding, env)
})
success <- TRUE

invisible(r_files)
Expand All @@ -42,10 +45,10 @@ load_code <- function(path = ".", quiet = NULL) {
find_code <- function(path = ".", quiet = FALSE) {
path_r <- package_file("R", path = path)

r_files <- withr_with_collate(
"C",
r_files <- local({
local_collate("C")
tools::list_files_with_type(path_r, "code", full.names = TRUE)
)
})

collate <- pkg_desc(path)$get_collate()

Expand Down
4 changes: 2 additions & 2 deletions R/load.R
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
package <- pkg_name(path)
description <- pkg_desc(path)

withr::local_envvar(c(DEVTOOLS_LOAD = package))
local_envvar(DEVTOOLS_LOAD = package)

quiet <- load_all_quiet(quiet, "load_all")

Expand All @@ -146,7 +146,7 @@
# JIT compilation and it would be locked before we can insert shims into
# it).
oldEnabled <- compiler::enableJIT(0)
on.exit(compiler::enableJIT(oldEnabled), TRUE)
defer(compiler::enableJIT(oldEnabled))

Check warning on line 149 in R/load.R

View check run for this annotation

Codecov / codecov/patch

R/load.R#L149

Added line #L149 was not covered by tests
}

if (missing(compile) && !missing(recompile)) {
Expand Down
6 changes: 2 additions & 4 deletions R/package-env.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ populate_pkg_env <- function(pkg,

# Source test helpers into pkg environment
if (helpers && uses_testthat(path)) {
withr_with_envvar(
c(NOT_CRAN = "true"),
testthat_source_test_helpers(find_test_dir(path), env = pkg_env)
)
local_envvar(NOT_CRAN = "true")
testthat_source_test_helpers(find_test_dir(path), env = pkg_env)
}
}

Expand Down
7 changes: 4 additions & 3 deletions R/source.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ source_many <- function(files, encoding = "UTF-8", envir = parent.frame()) {
stopifnot(is.character(files))
stopifnot(is.environment(envir))

oop <- options(

local_options(
keep.source = TRUE,
show.error.locations = TRUE,
topLevelEnvironment = as.environment(envir))
on.exit(options(oop))
topLevelEnvironment = as.environment(envir)
)

for (file in files) {
try_fetch(
Expand Down
3 changes: 2 additions & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ strip_internal_calls <- function(x, package) {
}

sort_ci <- function(x) {
withr_with_collate("C", x[order(tolower(x), x)])
local_collate("C")
x[order(tolower(x), x)]
}

dev_packages <- function() {
Expand Down
48 changes: 48 additions & 0 deletions R/withr-shims.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
defer <- function (expr, env = caller_env(), after = FALSE) {
thunk <- as.call(list(function() expr))
do.call(on.exit, list(thunk, TRUE, after), envir = env)
}

local_envvar <- function(..., .frame = parent.frame()) {
old <- set_envvar(list(...))
defer(set_envvar(old), .frame)

invisible()
}

local_collate <- function(locale, frame = parent.frame()) {
old <- Sys.getlocale("LC_COLLATE")
defer(Sys.setlocale("LC_COLLATE", old), frame)
Sys.setlocale("LC_COLLATE", locale)

# From https://github.com/r-lib/withr/blob/v3.0.0/R/locale.R#L51-L55:
# R supports setting LC_COLLATE to C via envvar. When that is the
# case, it takes precedence over the currently set locale. We need
# to set both the envvar and the locale for collate to fully take
# effect.
local_envvar(LC_COLLATE = locale, .frame = frame)

invisible()
}

local_dir <- function(path, frame = parent.frame()) {
old <- setwd(path)
defer(setwd(old), frame)

invisible()
}

# adapted from withr:::set_envvar
set_envvar <- function(envs) {
if (length(envs) == 0) {
return()

Check warning on line 38 in R/withr-shims.R

View check run for this annotation

Codecov / codecov/patch

R/withr-shims.R#L38

Added line #L38 was not covered by tests
}

old <- Sys.getenv(names(envs), names = TRUE, unset = NA)
set <- !is.na(envs)

if (any(set)) do.call("Sys.setenv", as.list(envs[set]))
if (any(!set)) Sys.unsetenv(names(envs)[!set])

invisible(old)
}
9 changes: 1 addition & 8 deletions R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,13 @@
}

# These functions are used in load_all() so need to exist in the
# devtools namespace so the withr namespace is not prematurely loaded
# devtools namespace so the desc namespace is not prematurely loaded
# by `::` during a load_all() call.
#
# They are lazily assigned to avoid racing issues while installing in
# parallel (see #89), and forced via `force_load_all_deps()` before
# unregistering namespaces.
on_load({
withr_with_dir %<~% withr::with_dir
withr_with_collate %<~% withr::with_collate
withr_with_envvar %<~% withr::with_envvar

desc_desc %<~% desc::desc
desc_desc_get_field %<~% desc::desc_get_field
desc_desc_get_version %<~% desc::desc_get_version
Expand All @@ -47,9 +43,6 @@ on_load({

force_load_all_deps <- function() {
list(
withr_with_dir,
withr_with_collate,
withr_with_envvar,
desc_desc,
desc_desc_get_field,
desc_desc_get_version,
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-dll.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ test_that("unload() unloads DLLs from packages loaded with library()", {
.libPaths(c(tmp_libpath, .libPaths()))

# Reset the libpath on exit
on.exit(.libPaths(old_libpaths), add = TRUE)
defer(.libPaths(old_libpaths))

# Install package
install.packages(test_path("testDllLoad"), repos = NULL, type = "source",
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-examples.R
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ test_that("run dontrun when requested", {

test_that("can run example package", {
load_all(test_path("testHelp"))
on.exit(unload(test_path("testHelp")))
defer(unload(test_path("testHelp")))

env <- dev_example("foofoo", quiet = TRUE)
expect_equal(env$a, 101)
})

test_that("can use system macros", {
load_all(test_path("testHelp"))
on.exit(unload(test_path("testHelp")))
defer(unload(test_path("testHelp")))

expect_silent(
run_example(
Expand Down
18 changes: 6 additions & 12 deletions tests/testthat/test-help.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ test_that("shim_question behaves the same as utils::? for nonexistent objects",

test_that("show_help and shim_question files for devtools-loaded packages", {
load_all(test_path('testHelp'))
on.exit(unload(test_path('testHelp')))
defer(unload(test_path('testHelp')))

h1 <- shim_help("foofoo")
expect_s3_class(h1, "dev_topic")
Expand All @@ -77,16 +77,13 @@ test_that("show_help and shim_question files for devtools-loaded packages", {
expect_equal(title, "testHelp:foofoo.Rd")
}

withr::with_options(
c(pager = pager_fun),
suppressMessages(
print(h1, type = 'text')
))
local_options(pager = pager_fun)
suppressMessages(print(h1, type = 'text'))
})

test_that("shim_help and shim_questions works if topic moves", {
load_all(test_path('testHelp'))
on.exit(unload(test_path('testHelp')))
defer(unload(test_path('testHelp')))

path_man <- test_path("testHelp/man/")
base_rd_path <- function(x) basename(x$path)
Expand All @@ -95,18 +92,15 @@ test_that("shim_help and shim_questions works if topic moves", {
expect_equal(base_rd_path(shim_question("foofoo")), "foofoo.Rd")

fs::file_move(fs::path(path_man, "foofoo.Rd"), fs::path(path_man, "barbar.Rd"))
on.exit(
fs::file_move(fs::path(path_man, "barbar.Rd"), fs::path(path_man, "foofoo.Rd")),
add = TRUE
)
defer(fs::file_move(fs::path(path_man, "barbar.Rd"), fs::path(path_man, "foofoo.Rd")))

expect_equal(base_rd_path(shim_help("foofoo")), "barbar.Rd")
expect_equal(base_rd_path(shim_question("foofoo")), "barbar.Rd")
})

test_that("dev_help works with package and function help with the same name", {
load_all(test_path('testHelp'))
on.exit(unload(test_path('testHelp')))
defer(unload(test_path('testHelp')))

h1 <- dev_help("testHelp")
expect_identical(shim_question(testHelp::testHelp), h1)
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-load.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ test_that("unloading or reloading forces bindings", {
withCallingHandlers(
forced = function(...) forced <<- TRUE, {
# Allow running test interactively
on.exit(unload("testLoadLazy"))
defer(unload("testLoadLazy"))

# On older R versions, `env_coalesce()` forces bindings
attach <- getRversion() >= "4.0.0"
Expand All @@ -77,7 +77,7 @@ test_that("unloading or reloading forces bindings", {
})

test_that("unloading or reloading does not call active bindings", {
on.exit(unload("testActiveBindings"))
defer(unload("testActiveBindings"))

expect_no_error(load_all(test_path("testActiveBindings")))
})
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-namespace-env.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
test_that("respects version separator", {
ns <- create_ns_env(test_path("testVersionSep"))
withr::defer(unregister_namespace("testVersionSep"))
defer(unregister_namespace("testVersionSep"))

expect_equal(getNamespaceInfo(ns, "spec")[["version"]], "0.0.0-9000")
})
4 changes: 2 additions & 2 deletions tests/testthat/test-po.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ test_that("translation domain correctly loaded", {
skip_if_c_locale()

load_all(test_path("testTranslations"))
withr::defer(unload("testTranslations"))
defer(unload("testTranslations"))

expect_equal(withr::with_language("fr", hello()), "Bonjour")

Expand All @@ -20,7 +20,7 @@ test_that("modified translations are correctly reloaded", {

# Load package and generate translation
load_all(pkg)
withr::defer(unload("testTranslations"))
defer(unload("testTranslations"))
withr::with_language("fr", hello())

# Modify .po file
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-shim.R
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ test_that("shim_library.dynam loads compiled dll/so from inst/src/", {

old_libpaths <- .libPaths()
.libPaths(c(tmp_libpath, .libPaths()))
on.exit(.libPaths(old_libpaths), add = TRUE)
defer(.libPaths(old_libpaths))

# Create temp directory for assembling testLibDynam with dll or so in inst/libs/
temp_dir <-tempdir()
Expand Down
Loading