From 9b412235d0d8a804603637ccecff98e880003feb Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Thu, 23 Jan 2025 07:40:11 -0600 Subject: [PATCH] Prevent `new_device = FALSE` from accidentally opening a device (#235) Because `par("page")` will open a device if one is not already open. Fixes #234. --- NEWS.md | 2 ++ R/watchout.R | 6 ++++++ tests/testthat/test-conditions.R | 9 ++++++--- tests/testthat/test-evaluate.R | 9 +++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 4198afdf..42384cd0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # evaluate (development version) +* `evaluate()` once again doesn't open a device if `new_device = FALSE` (#234) + # evaluate 1.0.3 # evaluate 1.0.2 diff --git a/R/watchout.R b/R/watchout.R index 796fb49f..80fe95d2 100644 --- a/R/watchout.R +++ b/R/watchout.R @@ -2,6 +2,7 @@ watchout <- function(handler = new_output_handler(), new_device = TRUE, debug = FALSE, frame = parent.frame()) { + if (new_device) { # Ensure we have a graphics device available for recording, but choose # one that's available on all platforms and doesn't write to disk @@ -43,6 +44,11 @@ watchout <- function(handler = new_output_handler(), sink_con <- local_persistent_sink_connection(debug, frame) capture_plot <- function(incomplete = FALSE) { + # no plots open; par("page") will open a device + if (is.null(dev.list())) { + return() + } + # only record plots for our graphics device if (!identical(dev.cur(), dev)) { return() diff --git a/tests/testthat/test-conditions.R b/tests/testthat/test-conditions.R index ead11dd2..2a303b4b 100644 --- a/tests/testthat/test-conditions.R +++ b/tests/testthat/test-conditions.R @@ -142,9 +142,12 @@ test_that("Error can be entraced and correctly handled in outputs", { skip_if_not_installed("knitr") skip_if_not_installed("callr") skip_on_cran() - # install dev version of package in temp directory - withr::local_temp_libpaths() - quick_install(pkgload::pkg_path("."), lib = .libPaths()[1]) + + # if not inside of R CMD check, install dev version into temp directory + if (Sys.getenv("_R_CHECK_TIMINGS_") == "") { + withr::local_temp_libpaths() + quick_install(pkgload::pkg_path("."), lib = .libPaths()[1]) + } out <- withr::local_tempfile(fileext = ".txt") diff --git a/tests/testthat/test-evaluate.R b/tests/testthat/test-evaluate.R index 7c6969c6..2cfb5031 100644 --- a/tests/testthat/test-evaluate.R +++ b/tests/testthat/test-evaluate.R @@ -121,6 +121,15 @@ test_that("check_keep can integrate log option", { expect_false(check_keep(FALSE, log = TRUE)$silence) }) +test_that("new_device = FALSE doesn't open any devices", { + graphics.off() + skip_if_not(is.null(dev.list())) + + ev <- evaluate("1", new_device = FALSE) + expect_equal(dev.list(), NULL) +}) + + test_that("check_keep errors with bad inputs", { expect_snapshot(error = TRUE, { check_keep(1, "keep_message")