diff --git a/NEWS.md b/NEWS.md index 32b7ca44f..550de611e 100644 --- a/NEWS.md +++ b/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 diff --git a/R/mock.R b/R/mock.R index 076cf9b50..c81ba7a2d 100644 --- a/R/mock.R +++ b/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( diff --git a/cran-comments.md b/cran-comments.md index 5c3db95b3..f7fab3dab 100644 --- a/cran-comments.md +++ b/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. diff --git a/man/with_mock.Rd b/man/with_mock.Rd index be3700a26..0ce31120d 100644 --- a/man/with_mock.Rd +++ b/man/with_mock.Rd @@ -24,20 +24,13 @@ For expert use only.} The result of the last unnamed parameter } \description{ -\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#superseded}{\figure{lifecycle-superseded.svg}{options: alt='[Superseded]'}}}{\strong{[Superseded]}} +\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} -\code{with_mock()} and \code{local_mock()} are superseded in favour of +\code{with_mock()} and \code{local_mock()} are deprecated in favour of \code{\link[=with_mocked_bindings]{with_mocked_bindings()}} and \code{\link[=local_mocked_bindings]{local_mocked_bindings()}}. -These works by using some C code to temporarily modify the mocked function -\emph{in place}. This is abusive of R's internals, which makes it dangerous, and -no longer recommended. -} -\section{3rd edition}{ - -\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} - -\code{with_mock()} and \code{local_mock()} are deprecated in the third edition. +These functions worked by using some C code to temporarily modify the mocked +function \emph{in place}. This was an abuse of R's internals and it is no longer +permitted. } - \keyword{internal} diff --git a/tests/testthat/_snaps/mock.md b/tests/testthat/_snaps/mock.md new file mode 100644 index 000000000..05f6e79af --- /dev/null +++ b/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. + diff --git a/tests/testthat/test-mock.R b/tests/testthat/test-mock.R index 8b229b5e6..dda947021 100644 --- a/tests/testthat/test-mock.R +++ b/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)) })