Deprecate with_mock() and local_mock()

NEWS.md

@@ -1,5 +1,6 @@
 # testthat (development version)
 
+* `with_mock()` and `local_mock()` have been unconditionally deprecated as they will no longer work in future versions of R (#1999).
 * `expect_condition()` and friends now include the `class` of the expected condition in the failure mesage, if used (#1987).
 * `LANGUAGE` is now set to `"C"` in reprocucible environments (i.e.
   `test_that()` blocks) to disable translations. This fixes warnings

R/mock.R

@@ -1,19 +1,14 @@
 #' Mock functions in a package.
 #'
 #' @description
-#' `r lifecycle::badge("superseded")`
+#' `r lifecycle::badge("deprecated")`
 #'
-#' `with_mock()` and `local_mock()` are superseded in favour of
+#' `with_mock()` and `local_mock()` are deprecated in favour of
 #' [with_mocked_bindings()] and [local_mocked_bindings()].
 #'
-#' These works by using some C code to temporarily modify the mocked function
-#' _in place_. This is abusive of R's internals, which makes it dangerous, and
-#' no longer recommended.
-#'
-#' @section 3rd edition:
-#' `r lifecycle::badge("deprecated")`
-#'
-#' `with_mock()` and `local_mock()` are deprecated in the third edition.
+#' These functions worked by using some C code to temporarily modify the mocked
+#' function _in place_. This was an abuse of R's internals and it is no longer
+#' permitted.
 #'
 #' @param ... named parameters redefine mocked functions, unnamed parameters
 #'   will be evaluated after mocking the functions
@@ -26,7 +21,7 @@
 #' @return The result of the last unnamed parameter
 #' @export
 with_mock <- function(..., .env = topenv()) {
-  edition_deprecate(3, "with_mock()", "Please use with_mocked_bindings() instead")
+  lifecycle::deprecate_warn("3.3.0", "with_mock()", "with_mocked_bindings()")
 
   dots <- eval(substitute(alist(...)))
   mock_qual_names <- names(dots)
@@ -61,7 +56,7 @@ with_mock <- function(..., .env = topenv()) {
 #' @export
 #' @rdname with_mock
 local_mock <- function(..., .env = topenv(), .local_envir = parent.frame()) {
-  edition_deprecate(3, "local_mock()", "Please use local_mocked_bindings() instead")
+  lifecycle::deprecate_warn("3.3.0", "local_mock()", "local_mocked_bindings()")
 
   mocks <- extract_mocks(list(...), .env = .env)
   on_exit <- bquote(

cran-comments.md

@@ -1,3 +1,12 @@
+## Check notes
+
+There is one check note in this version:
+
+ File ‘testthat/libs/testthat.so’:
+    Found non-API calls to R: ‘SET_BODY’, ‘SET_CLOENV’, ‘SET_FORMALS’
+
+The plan is to remove these calls in the next release, but I wanted to deprecated the problematic functions `with_mock()` and `local_mock()` first so that users get a little warning.
+
 ## revdepcheck results
 
 We checked all reverse dependencies, comparing R CMD check results across CRAN and dev versions of this package. Unfortunately something is up with our revdep test system and I failed to check ~1200 packages. I'm pretty confident these are bioconductor packages and unrelated to changes to testthat.

tests/testthat/_snaps/mock.md

@@ -0,0 +1,18 @@
+# deprecated
+
+    Code
+      local_mock()
+    Condition
+      Warning:
+      `local_mock()` was deprecated in testthat 3.3.0.
+      i Please use `local_mocked_bindings()` instead.
+
+---
+
+    Code
+      with_mock(is_testing = function() FALSE)
+    Condition
+      Warning:
+      `with_mock()` was deprecated in testthat 3.3.0.
+      i Please use `with_mocked_bindings()` instead.
+

tests/testthat/test-mock.R

@@ -1,150 +1,4 @@
-test_that("deprecated in 3rd edition", {
-  expect_warning(local_mock(), "deprecated")
-  expect_warning(with_mock(is_testing = function() FALSE), "deprecated")
-})
-
-test_that("can change value of internal function", {
-  local_edition(2)
-
-  with_mock(
-    test_mock_internal2 = function() 5,
-    expect_equal(test_mock_internal(), 5)
-  )
-
-  # and value is restored on error
-  expect_error(
-    with_mock(
-      test_mock_internal2 = function() 5,
-      stop("!")
-    )
-  )
-  expect_equal(test_mock_internal(), "y")
-})
-
-
-test_that("mocks can access local variables", {
-  local_edition(2)
-  x <- 5
-
-  with_mock(
-    test_mock_internal2 = function() x,
-    expect_equal(test_mock_internal(), 5)
-  )
-})
-
-test_that("non-empty mock with return value", {
-  local_edition(2)
-  expect_true(with_mock(
-    compare = function(x, y, ...) list(equal = TRUE, message = "TRUE"),
-    TRUE
-  ))
-})
-
-test_that("nested mock", {
-  local_edition(2)
-  with_mock(
-    all.equal = function(x, y, ...) TRUE,
-    {
-      with_mock(
-        expect_warning = expect_error,
-        {
-          expect_warning(stopifnot(!compare(3, "a")$equal))
-        }
-      )
-    },
-    .env = asNamespace("base")
-  )
-  expect_false(isTRUE(all.equal(3, 5)))
-  expect_warning(warning("test"))
-})
-
-test_that("can't mock non-existing", {
-  local_edition(2)
-  expect_error(with_mock(..bogus.. = identity, TRUE), "Function [.][.]bogus[.][.] not found in environment testthat")
-})
-
-test_that("can't mock non-function", {
-  local_edition(2)
-  expect_error(with_mock(pkg_and_name_rx = FALSE, TRUE), "Function pkg_and_name_rx not found in environment testthat")
-})
-
-test_that("empty or no-op mock", {
-  local_edition(2)
-  expect_warning(
-    expect_null(with_mock()),
-    "Not mocking anything. Please use named parameters to specify the functions you want to mock.",
-    fixed = TRUE
-  )
-  expect_warning(
-    expect_true(with_mock(TRUE)),
-    "Not mocking anything. Please use named parameters to specify the functions you want to mock.",
-    fixed = TRUE
-  )
-})
-
-test_that("visibility", {
-  local_edition(2)
-  expect_warning(expect_false(withVisible(with_mock())$visible))
-  expect_true(withVisible(with_mock(compare = function() {}, TRUE))$visible)
-  expect_false(withVisible(with_mock(compare = function() {}, invisible(5)))$visible)
-})
-
-test_that("multiple return values", {
-  local_edition(2)
-  expect_true(with_mock(FALSE, TRUE, compare = function() {}))
-  expect_equal(with_mock(3, compare = function() {}, 5), 5)
-})
-
-test_that("can access variables defined in function", {
-  local_edition(2)
-  x <- 5
-  expect_equal(with_mock(x, compare = function() {}), 5)
-})
-
-test_that("can mock if package is not loaded", {
-  local_edition(2)
-  if ("package:curl" %in% search()) {
-    skip("curl is loaded")
-  }
-  skip_if_not_installed("curl")
-  with_mock(`curl::curl` = identity, expect_identical(curl::curl, identity))
-})
-
-test_that("changes to variables are preserved between calls and visible outside", {
-  local_edition(2)
-  x <- 1
-  with_mock(
-    show_menu = function() {},
-    x <- 3,
-    expect_equal(x, 3)
-  )
-  expect_equal(x, 3)
-})
-
-test_that("mock extraction", {
-  local_edition(2)
-  expect_identical(
-    extract_mocks(list(compare = compare), .env = asNamespace("testthat"))$compare$name,
-    as.name("compare")
-  )
-  expect_error(
-    extract_mocks(list(..bogus.. = identity), "testthat"),
-    "Function [.][.]bogus[.][.] not found in environment testthat"
-  )
-  expect_equal(
-    length(extract_mocks(list(not = identity, show_menu = identity), "testthat")),
-    2
-  )
-})
-# local_mock --------------------------------------------------------------
-
-test_that("local_mock operates locally", {
-  local_edition(2)
-  f <- function() {
-    local_mock(compare = function(x, y) FALSE)
-    compare(1, 1)
-  }
-
-  expect_false(f())
-  expect_equal(compare(1, 1), no_difference())
+test_that("deprecated", {
+  expect_snapshot(local_mock())
+  expect_snapshot(with_mock(is_testing = function() FALSE))
 })