Skip to content

Commit

Permalink
Deprecate with_mock() and local_mock() (#2005)
Browse files Browse the repository at this point in the history
Fixes #1999
  • Loading branch information
hadley authored Oct 29, 2024
1 parent bf09659 commit 0d1d1a3
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 173 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
19 changes: 7 additions & 12 deletions R/mock.R
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions cran-comments.md
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 5 additions & 12 deletions man/with_mock.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions tests/testthat/_snaps/mock.md
Original file line number Diff line number Diff line change
@@ -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.

152 changes: 3 additions & 149 deletions tests/testthat/test-mock.R
Original file line number Diff line number Diff line change
@@ -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))
})

0 comments on commit 0d1d1a3

Please sign in to comment.