set_state_inspector() to compare global state before and after each test (#1816)

Fixes #1674

Author: hadley

NAMESPACE

@@ -168,6 +168,7 @@ export(quasi_label)
 export(run_cpp_tests)
 export(set_max_fails)
 export(set_reporter)
+export(set_state_inspector)
 export(setup)
 export(show_failure)
 export(shows_message)

NEWS.md

@@ -1,5 +1,13 @@
 # testthat (development version)
 
+* `set_state_inspector()` allows to to register a function that's called
+  before and after every test, reporting on any differences. This
+  is very useful for detecting if any of your tests have made changes to 
+  global state (like options, env vars, or connections) (#1674). This 
+  function was inspired by renv's testing infrastructure.
+
+* Only report test files that take longer than a second (#1806).
+
 * New `expect_contains()` and `expect_in()` that works similarly to 
   `expect_true(all(expected %in% object))` or 
   `expect_true(all(object %in% expected))` but give more informative failure
@@ -29,7 +37,6 @@
 * testthat no longer truncates tracebacks and uses rlang's default tree
   display.
 
-
 # testthat 3.1.8
 
 * `expect_snapshot()` differences no longer use quotes.

R/test-state.R

@@ -0,0 +1,76 @@
+#' State inspected
+#'
+#' @description
+#' One of the most pernicious challenges to debug is when a test runs fine
+#' in your test suite, but fails when you run it interactively (or similarly,
+#' it fails randomly when running your tests in parallel). One of the most
+#' common causes of this problem is accidentally changing global state in a
+#' previous test (e.g. changing an option, an environment variable, or the
+#' working directory). This is hard to debug, because it's very hard to figure
+#' out which test made the change.
+#'
+#' Luckily testthat provides a tool to figure out if tests are changing global
+#' state. You can register a state inspector with `set_state_inspector()` and
+#' testthat will run it before and after each test, store the results, then
+#' report if there are any differences. For example, if you wanted to see if
+#' any of your tests were changing options or environment variables, you could
+#' put this code in `tests/testthat/helper-state.R`:
+#'
+#' ```R
+#' set_state_inspector(function() {
+#'   list(
+#'     options = options(),
+#'     envvars = Sys.getenv()
+#'   )
+#' })
+#' ```
+#'
+#' (You might discover other packages outside your control are changing
+#' the global state, in which case you might want to modify this function
+#' to ignore those values.)
+#'
+#' Other problems that can be troublesome to resolve are CRAN check notes that
+#' report things like connections being left open. You can easily debug
+#' that problem with:
+#'
+#' ```R
+#' set_state_inspector(function() {
+#'   getAllConnections()
+#' }
+#' ```
+#'
+#' @export
+#' @param callback Either a zero-argument function that returns an object
+#'   capturing global state that you're interested in, or `NULL`.
+set_state_inspector <- function(callback) {
+
+  if (!is.null(callback) && !(is.function(callback) && length(formals(callback)) == 0)) {
+    cli::cli_abort("{.arg callback} must be a zero-arg function, or NULL")
+  }
+
+  the$state_inspector <- callback
+  invisible()
+}
+
+testthat_state_condition <- function(before, after, call) {
+
+  diffs <- waldo_compare(before, after, x_arg = "before", y_arg = "after")
+
+  if (length(diffs) == 0) {
+    return(NULL)
+  }
+
+  srcref <- attr(call, "srcref")
+  warning_cnd(
+    message = c("Global state has changed:", set_names(format(diffs), "")),
+    srcref = srcref
+  )
+}
+
+inspect_state <- function() {
+  if (is.null(the$state_inspector)) {
+    NULL
+  } else {
+    the$state_inspector()
+  }
+}

R/test-that.R

@@ -192,6 +192,7 @@ test_code <- function(test, code, env, default_reporter, skip_on_empty = TRUE) {
 
   withr::local_options(testthat_topenv = test_env)
 
+  before <- inspect_state()
   tryCatch(
     withCallingHandlers(
       {
@@ -211,7 +212,14 @@ test_code <- function(test, code, env, default_reporter, skip_on_empty = TRUE) {
     # skip silently terminate code
     skip  = function(e) {}
   )
+  after <- inspect_state()
 
+  if (!is.null(test)) {
+    cnd <- testthat_state_condition(before, after, call = sys.call(-1))
+    if (!is.null(cnd)) {
+      register_expectation(cnd, 0)
+    }
+  }
 
   invisible(ok)
 }

_pkgdown.yml

@@ -52,6 +52,7 @@ reference:
   - is_testing
   - skip
   - teardown_env
+  - set_state_inspector
 
 - title: Run tests
   contents:

man/set_state_inspector.Rd

@@ -0,0 +1,20 @@
+# set_state_inspector() verifies its inputs
+
+    Code
+      set_state_inspector(function(x) 123)
+    Condition
+      Error in `set_state_inspector()`:
+      ! `callback` must be a zero-arg function, or NULL
+
+# can detect state changes
+
+    [ FAIL 0 | WARN 1 | SKIP 0 | PASS 1 ]
+    
+    == Warnings ====================================================================
+    -- Warning ('reporters/state-change.R:1:1'): options ---------------------------
+    Global state has changed:
+    `before$x` is NULL
+    `after$x` is a double vector (1)
+    
+    [ FAIL 0 | WARN 1 | SKIP 0 | PASS 1 ]
+

tests/testthat/_snaps/test-state.md

@@ -0,0 +1,4 @@
+test_that("options", {
+  options(x = 1)
+  expect_true(TRUE)
+})

tests/testthat/reporters/state-change.R

@@ -1,6 +1,6 @@
 test_that("teardown adds to queue", {
   local_edition(2)
-  on.exit(teardown_reset())
+  withr::defer({teardown_reset()})
 
   expect_length(file_teardown_env$queue, 0)
 
@@ -13,7 +13,7 @@ test_that("teardown adds to queue", {
 
 test_that("teardowns runs in order", {
   local_edition(2)
-  on.exit(teardown_reset())
+  withr::defer(teardown_reset())
 
   a <- 1
   teardown(a <<- 2)

tests/testthat/test-teardown.R

@@ -0,0 +1,11 @@
+test_that("set_state_inspector() verifies its inputs", {
+  expect_snapshot(set_state_inspector(function(x) 123), error = TRUE)
+})
+
+test_that("can detect state changes", {
+  local_options(x = NULL)
+  set_state_inspector(function() list(x = getOption("x")))
+  withr::defer(set_state_inspector(NULL))
+
+  expect_snapshot_reporter(CheckReporter$new(), test_path("reporters/state-change.R"))
+})