Skip to content

Commit fab6fdc

Browse files
hadleyhadley
authored
Always check base class in 3e condition expectations (#1770)
Fixes #1668
1 parent d4dd3ab commit fab6fdc

File tree

9 files changed

+3414
-736
lines changed

9 files changed

+3414
-736
lines changed

‎NEWS.md‎

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

3+
* `expect_error()`, `expect_warning()`, and `expect_message()` now correctly
4+
enforce that the condition is of the expected base class (e.g. error,
5+
warning, messsage) even when the `class` argument is used (#1168).
6+
37
* `source_file()`, which is used by various parts of the helper and
48
setup/teardown machinery, now reports the file name in the case of
59
errors (#1704).

‎R/expect-condition.R‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,13 @@ expect_condition_matching <- function(base_class,
258258
ellipsis::check_dots_used(action = warn)
259259

260260
matcher <- cnd_matcher(
261-
class %||% base_class,
261+
base_class,
262+
class,
262263
regexp,
263264
...,
264265
inherit = inherit,
265-
ignore_deprecation = base_class == "warning" && identical(regexp, NA)
266+
ignore_deprecation = base_class == "warning" && identical(regexp, NA),
267+
error_call = trace_env
266268
)
267269

268270
act <- quasi_capture(
@@ -286,13 +288,15 @@ expect_condition_matching <- function(base_class,
286288

287289
# -------------------------------------------------------------------------
288290

289-
cnd_matcher <- function(class, pattern = NULL, ..., inherit = TRUE, ignore_deprecation = FALSE) {
290-
if (!is_string(class)) {
291-
abort("`class` must be a single string")
292-
}
293-
if (!is_string(pattern) && !is.null(pattern) && !isNA(pattern)) {
294-
abort("`pattern` must be a single string, NULL, or NA")
295-
}
291+
cnd_matcher <- function(base_class,
292+
class = NULL,
293+
pattern = NULL,
294+
...,
295+
inherit = TRUE,
296+
ignore_deprecation = FALSE,
297+
error_call = caller_env()) {
298+
check_string(class, allow_null = TRUE, call = error_call)
299+
check_string(pattern, allow_null = TRUE, allow_na = TRUE, call = error_call)
296300

297301
function(cnd) {
298302
if (!inherit) {
@@ -303,26 +307,31 @@ cnd_matcher <- function(class, pattern = NULL, ..., inherit = TRUE, ignore_depre
303307
return(FALSE)
304308
}
305309

306-
if (is.null(pattern) || isNA(pattern)) {
307-
cnd_inherits(cnd, class)
308-
} else {
309-
cnd_matches(cnd, class, pattern, ...)
310+
matcher <- function(x) {
311+
if (!inherits(x, base_class)) {
312+
return(FALSE)
313+
}
314+
if (!is.null(class) && !inherits(x, class)) {
315+
return(FALSE)
316+
}
317+
if (!is.null(pattern) && !isNA(pattern)) {
318+
grepl(pattern, conditionMessage(x), ...)
319+
} else {
320+
TRUE
321+
}
310322
}
323+
cnd_some(cnd, matcher)
311324
}
312325
}
313326

327+
has_classes <- function(x, classes) {
328+
all(classes %in% class(x))
329+
}
330+
314331
is_deprecation <- function(x) {
315332
inherits(x, "lifecycle_warning_deprecated")
316333
}
317334

318-
cnd_inherits <- function(cnd, class) {
319-
cnd_some(cnd, ~ inherits(.x, class))
320-
}
321-
cnd_matches <- function(cnd, class, pattern, ...) {
322-
cnd_some(cnd, function(x) {
323-
inherits(x, class) && grepl(pattern, conditionMessage(x), ...)
324-
})
325-
}
326335
cnd_some <- function(.cnd, .p, ...) {
327336
.p <- as_function(.p)
328337

‎R/expect-no-condition.R‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ expect_no_ <- function(base_class,
9090
error_call = caller_env()) {
9191

9292
matcher <- cnd_matcher(
93-
class %||% base_class,
93+
base_class,
94+
class,
9495
pattern = regexp,
9596
ignore_deprecation = base_class == "warning" && is.null(regexp) && is.null(class)
9697
)

‎revdep/README.md‎

Lines changed: 93 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,99 @@
11
# Revdeps
22

3-
## Failed to check (31)
3+
## Failed to check (75)
44

5-
|package |version |error |warning |note |
6-
|:------------|:-------|:-----|:-------|:----|
7-
|Boom |0.9.10 |1 | | |
8-
|CausalImpact |? | | | |
9-
|NA |? | | | |
10-
|ctsem |3.7.1 |1 | | |
11-
|NA |? | | | |
12-
|elbird |0.2.5 |1 | | |
13-
|ggPMX |? | | | |
14-
|NA |? | | | |
15-
|NA |? | | | |
16-
|loon.ggplot |? | | | |
17-
|loon.shiny |? | | | |
18-
|loon.tourr |? | | | |
19-
|NA |? | | | |
20-
|NA |? | | | |
21-
|mlrCPO |0.3.7-4 |1 | | |
22-
|nlmixr2 |? | | | |
23-
|nlmixr2extra |? | | | |
24-
|nlmixr2plot |? | | | |
25-
|NA |? | | | |
26-
|OpenMx |? | | | |
27-
|NA |? | | | |
28-
|rPBK |0.2.0 |1 | | |
29-
|NA |? | | | |
30-
|NA |? | | | |
31-
|SSVS |? | | | |
32-
|NA |? | | | |
33-
|NA |? | | | |
34-
|tidySEM |? | | | |
35-
|NA |? | | | |
36-
|vivid |? | | | |
37-
|NA |? | | | |
5+
|package |version |error |warning |note |
6+
|:----------------|:--------|:-----|:-------|:----|
7+
|babelmixr2 |? | | | |
8+
|beadplexr |? | | | |
9+
|Boom |0.9.11 |1 | | |
10+
|bsts |? | | | |
11+
|CausalImpact |? | | | |
12+
|convey |? | | | |
13+
|covidmx |? | | | |
14+
|ctbi |? | | | |
15+
|ctsem |3.7.6 |1 | | |
16+
|D2MCS |? | | | |
17+
|DAISIE |? | | | |
18+
|ddc |? | | | |
19+
|discAUC |? | | | |
20+
|ebvcube |? | | | |
21+
|emdi |? | | | |
22+
|EpiForsk |? | | | |
23+
|epitopeR |? | | | |
24+
|EScvtmle |? | | | |
25+
|fdaPDE |1.1-16 |1 | | |
26+
|forceR |? | | | |
27+
|genekitr |? | | | |
28+
|ggpicrust2 |? | | | |
29+
|ggPMX |? | | | |
30+
|gllvm |1.4.1 |1 | | |
31+
|journalabbr |? | | | |
32+
|landsepi |? | | | |
33+
|lifeR |? | | | |
34+
|LikertMakeR |? | | | |
35+
|loon.ggplot |? | | | |
36+
|loon.shiny |? | | | |
37+
|loon.tourr |? | | | |
38+
|mcglm |? | | | |
39+
|MGMS2 |1.0.2 |1 | | |
40+
|microservices |? | | | |
41+
|moexer |? | | | |
42+
|MRTAnalysis |? | | | |
43+
|nlmixr2 |? | | | |
44+
|nlmixr2extra |? | | | |
45+
|nlmixr2lib |? | | | |
46+
|nlmixr2plot |? | | | |
47+
|nlmixr2rpt |? | | | |
48+
|numbat |? | | | |
49+
|OlinkAnalyze |? | | | |
50+
|oosse |? | | | |
51+
|OpenMx |? | | | |
52+
|paramsim |? | | | |
53+
|pathwayTMB |? | | | |
54+
|prqlr |? | | | |
55+
|rankinma |? | | | |
56+
|RevGadgets |? | | | |
57+
|rlibkriging |? | | | |
58+
|rolog |? | | | |
59+
|rswipl |? | | | |
60+
|rTensor2 |? | | | |
61+
|SCpubr |? | | | |
62+
|SherlockHolmes |? | | | |
63+
|shinyHugePlot |? | | | |
64+
|string2path |? | | | |
65+
|symengine |? | | | |
66+
|tame |? | | | |
67+
|taxlist |? | | | |
68+
|TempStable |? | | | |
69+
|textBoxPlacement |? | | | |
70+
|tidySEM |? | | | |
71+
|tidytags |? | | | |
72+
|timeLineGraphics |? | | | |
73+
|tinyarray |? | | | |
74+
|vegtable |? | | | |
75+
|vivid |? | | | |
76+
|VSURF |? | | | |
77+
|waspasR |? | | | |
78+
|Wats |? | | | |
79+
|wrappedtools |? | | | |
80+
|xtensor |0.14.1-0 |1 | | |
81+
|yamlme |? | | | |
3882

39-
## New problems (2)
83+
## New problems (12)
4084

41-
|package |version |error |warning |note |
42-
|:------------|:-------|:------|:-------|:----|
43-
|[datagovindia](problems.md#datagovindia)|1.0.5 | |__+1__ | |
44-
|[hyperSpec](problems.md#hyperspec)|0.100.0 |__+1__ | |1 |
85+
|package |version |error |warning |note |
86+
|:-----------|:-------|:--------|:-------|:----|
87+
|[bookdown](problems.md#bookdown)|0.33 |__+1__ | | |
88+
|[crosstable](problems.md#crosstable)|0.6.1 |__+1__ | | |
89+
|[extras](problems.md#extras)|0.5.0 |__+1__ | | |
90+
|[fs](problems.md#fs)|1.6.1 |__+1__ | |1 |
91+
|[gfpop](problems.md#gfpop)|1.1.1 |__+1__ | | |
92+
|[gh](problems.md#gh)|1.4.0 |__+1__ | | |
93+
|[lightgbm](problems.md#lightgbm)|3.3.5 |__+1__ | |1 |
94+
|[mpathsenser](problems.md#mpathsenser)|1.1.3 |__+1__ | | |
95+
|[PKNCA](problems.md#pknca)|0.10.1 |__+1__ | | |
96+
|[rlang](problems.md#rlang)|1.1.0 |__+1__ | |1 |
97+
|[rtry](problems.md#rtry)|1.0.0 |1 __+1__ | |2 |
98+
|[tidyquant](problems.md#tidyquant)|1.0.7 |__+1__ | |1 |
4599

‎revdep/cran.md‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,81 @@
11
## revdepcheck results
22

3-
We did not check reverse dependencies, since this is a small patch release.
3+
We checked 7435 reverse dependencies (7391 from CRAN + 44 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package.
4+
5+
* We saw 12 new problems
6+
* We failed to check 31 packages
7+
8+
Issues with CRAN packages are summarised below.
9+
10+
### New problems
11+
(This reports the first line of each new failure)
12+
13+
* bookdown
14+
checking tests ... ERROR
15+
16+
* crosstable
17+
checking tests ... ERROR
18+
19+
* extras
20+
checking tests ... ERROR
21+
22+
* fs
23+
checking tests ... ERROR
24+
25+
* gfpop
26+
checking tests ... ERROR
27+
28+
* gh
29+
checking tests ... ERROR
30+
31+
* lightgbm
32+
checking tests ... ERROR
33+
34+
* mpathsenser
35+
checking tests ... ERROR
36+
37+
* PKNCA
38+
checking tests ... ERROR
39+
40+
* rlang
41+
checking tests ... ERROR
42+
43+
* rtry
44+
checking tests ... ERROR
45+
46+
* tidyquant
47+
checking examples ... ERROR
48+
49+
### Failed to check
50+
51+
* babelmixr2 (NA)
52+
* beadplexr (NA)
53+
* Boom (NA)
54+
* bsts (NA)
55+
* CausalImpact (NA)
56+
* ctsem (NA)
57+
* fdaPDE (NA)
58+
* genekitr (NA)
59+
* ggpicrust2 (NA)
60+
* ggPMX (NA)
61+
* gllvm (NA)
62+
* loon.ggplot (NA)
63+
* loon.shiny (NA)
64+
* loon.tourr (NA)
65+
* MGMS2 (NA)
66+
* nlmixr2 (NA)
67+
* nlmixr2extra (NA)
68+
* nlmixr2lib (NA)
69+
* nlmixr2plot (NA)
70+
* nlmixr2rpt (NA)
71+
* numbat (NA)
72+
* OlinkAnalyze (NA)
73+
* OpenMx (NA)
74+
* pathwayTMB (NA)
75+
* SCpubr (NA)
76+
* taxlist (NA)
77+
* tidySEM (NA)
78+
* tinyarray (NA)
79+
* vegtable (NA)
80+
* vivid (NA)
81+
* xtensor (NA)

0 commit comments

Comments
 (0)