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

@@ -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

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()

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")
 

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")