Skip to content

Commit 715c088

Browse files
hadleyhadley
authored
Merge branch 'main' into bugfix/testthat-empty-desc
2 parents 0c0efcb + c843a3b commit 715c088

File tree

8 files changed

+52
-175
lines changed

8 files changed

+52
-175
lines changed

‎NEWS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# testthat (development version)
22

33
* `expect_snapshot()` now errors when called from a `test_that()` that has an empty description (@kevinushey, #1980).
4+
* `skip_if_not_installed()` produces a clearer message (@MichaelChirico, #1959).
5+
* `with_mock()` and `local_mock()` have been unconditionally deprecated as they will no longer work in future versions of R (#1999).
46
* `expect_condition()` and friends now include the `class` of the expected condition in the failure mesage, if used (#1987).
57
* `LANGUAGE` is now set to `"C"` in reprocucible environments (i.e.
68
`test_that()` blocks) to disable translations. This fixes warnings

‎R/mock.R‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
#' Mock functions in a package.
22
#'
33
#' @description
4-
#' `r lifecycle::badge("superseded")`
4+
#' `r lifecycle::badge("deprecated")`
55
#'
6-
#' `with_mock()` and `local_mock()` are superseded in favour of
6+
#' `with_mock()` and `local_mock()` are deprecated in favour of
77
#' [with_mocked_bindings()] and [local_mocked_bindings()].
88
#'
9-
#' These works by using some C code to temporarily modify the mocked function
10-
#' _in place_. This is abusive of R's internals, which makes it dangerous, and
11-
#' no longer recommended.
12-
#'
13-
#' @section 3rd edition:
14-
#' `r lifecycle::badge("deprecated")`
15-
#'
16-
#' `with_mock()` and `local_mock()` are deprecated in the third edition.
9+
#' These functions worked by using some C code to temporarily modify the mocked
10+
#' function _in place_. This was an abuse of R's internals and it is no longer
11+
#' permitted.
1712
#'
1813
#' @param ... named parameters redefine mocked functions, unnamed parameters
1914
#' will be evaluated after mocking the functions
@@ -26,7 +21,7 @@
2621
#' @return The result of the last unnamed parameter
2722
#' @export
2823
with_mock <- function(..., .env = topenv()) {
29-
edition_deprecate(3, "with_mock()", "Please use with_mocked_bindings() instead")
24+
lifecycle::deprecate_warn("3.3.0", "with_mock()", "with_mocked_bindings()")
3025

3126
dots <- eval(substitute(alist(...)))
3227
mock_qual_names <- names(dots)
@@ -61,7 +56,7 @@ with_mock <- function(..., .env = topenv()) {
6156
#' @export
6257
#' @rdname with_mock
6358
local_mock <- function(..., .env = topenv(), .local_envir = parent.frame()) {
64-
edition_deprecate(3, "local_mock()", "Please use local_mocked_bindings() instead")
59+
lifecycle::deprecate_warn("3.3.0", "local_mock()", "local_mocked_bindings()")
6560

6661
mocks <- extract_mocks(list(...), .env = .env)
6762
on_exit <- bquote(

‎R/skip.R‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,14 @@ skip_if <- function(condition, message = NULL) {
100100
#' @param minimum_version Minimum required version for the package
101101
#' @rdname skip
102102
skip_if_not_installed <- function(pkg, minimum_version = NULL) {
103+
# most common case: it's not installed
104+
tryCatch(
105+
find.package(pkg),
106+
error = function(e) skip(paste0("{", pkg, "} is not installed"))
107+
)
108+
# rarer: it's installed, but fails to load
103109
if (!requireNamespace(pkg, quietly = TRUE)) {
104-
skip(paste0(pkg, " cannot be loaded"))
110+
skip(paste0("{", pkg, "} cannot be loaded"))
105111
}
106112

107113
if (!is.null(minimum_version)) {

‎cran-comments.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
## Check notes
2+
3+
There is one check note in this version:
4+
5+
File ‘testthat/libs/testthat.so’:
6+
Found non-API calls to R: ‘SET_BODY’, ‘SET_CLOENV’, ‘SET_FORMALS’
7+
8+
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.
9+
110
## revdepcheck results
211

312
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.

‎man/with_mock.Rd‎

Lines changed: 5 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/testthat/_snaps/mock.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# deprecated
2+
3+
Code
4+
local_mock()
5+
Condition
6+
Warning:
7+
`local_mock()` was deprecated in testthat 3.3.0.
8+
i Please use `local_mocked_bindings()` instead.
9+
10+
---
11+
12+
Code
13+
with_mock(is_testing = function() FALSE)
14+
Condition
15+
Warning:
16+
`with_mock()` was deprecated in testthat 3.3.0.
17+
i Please use `with_mocked_bindings()` instead.
18+

‎tests/testthat/_snaps/skip.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
# skip_if_not_installed() works as expected
2828

29-
Reason: doesntexist cannot be loaded
29+
Reason: {doesntexist} is not installed
3030

3131
---
3232

‎tests/testthat/test-mock.R‎

Lines changed: 3 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -1,150 +1,4 @@
1-
test_that("deprecated in 3rd edition", {
2-
expect_warning(local_mock(), "deprecated")
3-
expect_warning(with_mock(is_testing = function() FALSE), "deprecated")
4-
})
5-
6-
test_that("can change value of internal function", {
7-
local_edition(2)
8-
9-
with_mock(
10-
test_mock_internal2 = function() 5,
11-
expect_equal(test_mock_internal(), 5)
12-
)
13-
14-
# and value is restored on error
15-
expect_error(
16-
with_mock(
17-
test_mock_internal2 = function() 5,
18-
stop("!")
19-
)
20-
)
21-
expect_equal(test_mock_internal(), "y")
22-
})
23-
24-
25-
test_that("mocks can access local variables", {
26-
local_edition(2)
27-
x <- 5
28-
29-
with_mock(
30-
test_mock_internal2 = function() x,
31-
expect_equal(test_mock_internal(), 5)
32-
)
33-
})
34-
35-
test_that("non-empty mock with return value", {
36-
local_edition(2)
37-
expect_true(with_mock(
38-
compare = function(x, y, ...) list(equal = TRUE, message = "TRUE"),
39-
TRUE
40-
))
41-
})
42-
43-
test_that("nested mock", {
44-
local_edition(2)
45-
with_mock(
46-
all.equal = function(x, y, ...) TRUE,
47-
{
48-
with_mock(
49-
expect_warning = expect_error,
50-
{
51-
expect_warning(stopifnot(!compare(3, "a")$equal))
52-
}
53-
)
54-
},
55-
.env = asNamespace("base")
56-
)
57-
expect_false(isTRUE(all.equal(3, 5)))
58-
expect_warning(warning("test"))
59-
})
60-
61-
test_that("can't mock non-existing", {
62-
local_edition(2)
63-
expect_error(with_mock(..bogus.. = identity, TRUE), "Function [.][.]bogus[.][.] not found in environment testthat")
64-
})
65-
66-
test_that("can't mock non-function", {
67-
local_edition(2)
68-
expect_error(with_mock(pkg_and_name_rx = FALSE, TRUE), "Function pkg_and_name_rx not found in environment testthat")
69-
})
70-
71-
test_that("empty or no-op mock", {
72-
local_edition(2)
73-
expect_warning(
74-
expect_null(with_mock()),
75-
"Not mocking anything. Please use named parameters to specify the functions you want to mock.",
76-
fixed = TRUE
77-
)
78-
expect_warning(
79-
expect_true(with_mock(TRUE)),
80-
"Not mocking anything. Please use named parameters to specify the functions you want to mock.",
81-
fixed = TRUE
82-
)
83-
})
84-
85-
test_that("visibility", {
86-
local_edition(2)
87-
expect_warning(expect_false(withVisible(with_mock())$visible))
88-
expect_true(withVisible(with_mock(compare = function() {}, TRUE))$visible)
89-
expect_false(withVisible(with_mock(compare = function() {}, invisible(5)))$visible)
90-
})
91-
92-
test_that("multiple return values", {
93-
local_edition(2)
94-
expect_true(with_mock(FALSE, TRUE, compare = function() {}))
95-
expect_equal(with_mock(3, compare = function() {}, 5), 5)
96-
})
97-
98-
test_that("can access variables defined in function", {
99-
local_edition(2)
100-
x <- 5
101-
expect_equal(with_mock(x, compare = function() {}), 5)
102-
})
103-
104-
test_that("can mock if package is not loaded", {
105-
local_edition(2)
106-
if ("package:curl" %in% search()) {
107-
skip("curl is loaded")
108-
}
109-
skip_if_not_installed("curl")
110-
with_mock(`curl::curl` = identity, expect_identical(curl::curl, identity))
111-
})
112-
113-
test_that("changes to variables are preserved between calls and visible outside", {
114-
local_edition(2)
115-
x <- 1
116-
with_mock(
117-
show_menu = function() {},
118-
x <- 3,
119-
expect_equal(x, 3)
120-
)
121-
expect_equal(x, 3)
122-
})
123-
124-
test_that("mock extraction", {
125-
local_edition(2)
126-
expect_identical(
127-
extract_mocks(list(compare = compare), .env = asNamespace("testthat"))$compare$name,
128-
as.name("compare")
129-
)
130-
expect_error(
131-
extract_mocks(list(..bogus.. = identity), "testthat"),
132-
"Function [.][.]bogus[.][.] not found in environment testthat"
133-
)
134-
expect_equal(
135-
length(extract_mocks(list(not = identity, show_menu = identity), "testthat")),
136-
2
137-
)
138-
})
139-
# local_mock --------------------------------------------------------------
140-
141-
test_that("local_mock operates locally", {
142-
local_edition(2)
143-
f <- function() {
144-
local_mock(compare = function(x, y) FALSE)
145-
compare(1, 1)
146-
}
147-
148-
expect_false(f())
149-
expect_equal(compare(1, 1), no_difference())
1+
test_that("deprecated", {
2+
expect_snapshot(local_mock())
3+
expect_snapshot(with_mock(is_testing = function() FALSE))
1504
})

0 commit comments

Comments
 (0)