Skip to content

Commit 889de40

Browse files
committed
Ensure expect_no_*() runs complete block
Instead of terminating early. Fixes #1991.
1 parent ce394a5 commit 889de40

File tree

3 files changed

+54
-45
lines changed

3 files changed

+54
-45
lines changed

NEWS.md

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

3+
* `expect_no_*()` now executes the entire code block, rather than stopping at the first message or warning (#1991).
34
* `expect_no_failures()` and `expect_no_successes()` are now deprecated as `expect_success()` now test for no failures and `expect_failure()` tests for no successes (#)
45
* New `pass()` function to use in place of `succeed()` (#2113).
56
* `expectation()` is now a combination of `new_expectation()` and `exp_signal()` (#2125).

R/expect-no-condition.R

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -82,45 +82,53 @@ expect_no_ <- function(
8282
is.null(class)
8383
)
8484

85+
first_match <- NULL
8586
capture <- function(code) {
86-
try_fetch(
87-
{
88-
code
89-
# We can't call succeed() here because that generates a condition
90-
# that causes `expect_no_condition()` to always fail
91-
NULL
92-
},
93-
!!base_class := function(cnd) {
94-
if (!matcher(cnd)) {
95-
return(zap())
96-
}
87+
withRestarts(
88+
withCallingHandlers(
89+
code,
90+
condition = function(cnd) {
91+
if (!is.null(first_match) || !matcher(cnd)) {
92+
return()
93+
}
94+
95+
first_match <<- cnd
96+
cnd_muffle(cnd)
9797

98-
expected <- paste0(
99-
"Expected ",
100-
quo_label(enquo(object)),
101-
" to run without any ",
102-
base_class,
103-
"s",
104-
if (!is.null(class)) paste0(" of class '", class, "'"),
105-
if (!is.null(regexp)) paste0(" matching pattern '", regexp, "'"),
106-
"."
107-
)
108-
actual <- paste0(
109-
"Actually got a <",
110-
class(cnd)[[1]],
111-
"> with text:\n",
112-
indent_lines(rlang::cnd_message(cnd))
113-
)
114-
message <- format_error_bullets(c(expected, i = actual))
115-
fail(message, trace_env = trace_env)
116-
}
98+
# Matching errors should terminate execution
99+
if (inherits(cnd, "error")) {
100+
invokeRestart("done")
101+
}
102+
}
103+
),
104+
done = function() {}
117105
)
118106
}
119107

120108
act <- quasi_capture(enquo(object), NULL, capture)
121-
if (is.null(act$cap)) {
122-
succeed()
109+
110+
if (!is.null(first_match)) {
111+
expected <- paste0(
112+
"Expected ",
113+
quo_label(enquo(object)),
114+
" to run without any ",
115+
base_class,
116+
"s",
117+
if (!is.null(class)) paste0(" of class '", class, "'"),
118+
if (!is.null(regexp)) paste0(" matching pattern '", regexp, "'"),
119+
"."
120+
)
121+
actual <- paste0(
122+
"Actually got a <",
123+
class(first_match)[[1]],
124+
"> with text:\n",
125+
indent_lines(rlang::cnd_message(first_match))
126+
)
127+
message <- format_error_bullets(c(expected, i = actual))
128+
return(fail(message, trace_env = trace_env))
123129
}
130+
131+
succeed()
124132
invisible(act$val)
125133
}
126134

tests/testthat/test-expect-no-condition.R

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,26 @@ test_that("expect_no_* pass with pure code", {
1919
expect_success(expect_no_condition(1))
2020
})
2121

22+
test_that("expect_no_ continues execution", {
23+
b <- 1
24+
expect_failure(expect_no_warning({
25+
warning("x")
26+
b <- 2
27+
}))
28+
expect_equal(b, 2)
29+
})
30+
2231
test_that("expect_no_* don't emit success when they fail", {
2332
expect_failure(expect_no_error(stop("!")))
2433
})
2534

2635
test_that("capture correct trace_env (#1994)", {
2736
# This should fail, not error
28-
expect_failure(
29-
expect_message({
30-
message("a")
31-
warn("b")
32-
}) |>
33-
expect_no_warning()
34-
)
35-
expect_failure(
36-
expect_no_message({
37-
message("a")
38-
warn("b")
39-
}) |>
40-
expect_warning()
37+
status <- capture_success_failure(
38+
expect_warning(expect_error(stop("oops")))
4139
)
40+
expect_equal(status$n_success, 1) # from expect_error()
41+
expect_equal(status$n_failure, 1) # from expect_warning()
4242
})
4343

4444
test_that("unmatched conditions bubble up", {

0 commit comments

Comments
 (0)