From b506ba65469044d6520db58f69629444e902f0b0 Mon Sep 17 00:00:00 2001 From: ALanguillaume Date: Tue, 11 Jan 2022 12:45:24 +0100 Subject: [PATCH 1/2] Accept only existing editions An error is thrown when trying to set an edition that does not exist. Fixes #1547. --- R/edition.R | 12 +++++++++++- tests/testthat/test-edition.R | 11 +++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/R/edition.R b/R/edition.R index 9201e268a..b50b58f34 100644 --- a/R/edition.R +++ b/R/edition.R @@ -51,6 +51,14 @@ edition_name <- function(x) { } } +is_valid_edition <- function(x) { + if (is_zap(x)) { + TRUE + } else { + is.numeric(x) && length(x) == 1 && x %in% c(2, 3) + } +} + #' Temporarily change the active testthat edition #' #' `local_edition()` allows you to temporarily (within a single test or @@ -62,7 +70,9 @@ edition_name <- function(x) { #' @param .env Environment that controls scope of changes. For expert use only. #' @keywords internal local_edition <- function(x, .env = parent.frame()) { - stopifnot(is_zap(x) || (is.numeric(x) && length(x) == 1)) + if (!is_valid_edition(x)) { + stop("Available editions are 2 and 3", call. = FALSE) + } old <- edition_set(x) withr::defer(edition_set(old), envir = .env) } diff --git a/tests/testthat/test-edition.R b/tests/testthat/test-edition.R index 4fc4d19b1..5213ad22f 100644 --- a/tests/testthat/test-edition.R +++ b/tests/testthat/test-edition.R @@ -6,6 +6,17 @@ test_that("can locally override edition", { expect_equal(edition_get(), 2) }) +test_that("only existing editions can be set", { + expect_error( + local_edition(5), + regexp = "Available editions are 2 and 3" + ) + expect_error( + local_edition(-1), + regexp = "Available editions are 2 and 3" + ) +}) + test_that("deprecation only fired for newer edition", { local_edition(2) expect_warning(edition_deprecate(3, "old stuff"), NA) From 8c8964bddaf5f037f97f2a636ca0485282a3c540 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 25 Jul 2025 13:46:07 -0500 Subject: [PATCH 2/2] Use existing check functions --- NEWS.md | 1 + R/edition.R | 13 +------------ tests/testthat/_snaps/edition.md | 13 +++++++++++++ tests/testthat/test-edition.R | 17 +++++++---------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/NEWS.md b/NEWS.md index 3fa3f7e5d..78fd1ed9b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `local_edition()` now gives a useful error for bad values (#1547). * testthat now requires R 4.1. * `expect_s4_class()` now supports unquoting (@stibu81, #2064). * `it()` now finds the correct evaluation environment in more cases (@averissimo, #2085). diff --git a/R/edition.R b/R/edition.R index b4c83bff3..c2bf9c9dd 100644 --- a/R/edition.R +++ b/R/edition.R @@ -51,14 +51,6 @@ edition_name <- function(x) { } } -is_valid_edition <- function(x) { - if (is_zap(x)) { - TRUE - } else { - is.numeric(x) && length(x) == 1 && x %in% c(2, 3) - } -} - #' Temporarily change the active testthat edition #' #' `local_edition()` allows you to temporarily (within a single test or @@ -69,10 +61,7 @@ is_valid_edition <- function(x) { #' @param x Edition Should be a single integer. #' @param .env Environment that controls scope of changes. For expert use only. local_edition <- function(x, .env = parent.frame()) { - if (!is_valid_edition(x)) { - stop("Available editions are 2 and 3", call. = FALSE) - } - + check_number_whole(x, min = 2, max = 3) local_bindings(edition = x, .env = the, .frame = .env) } diff --git a/tests/testthat/_snaps/edition.md b/tests/testthat/_snaps/edition.md index da4619866..82cb92c69 100644 --- a/tests/testthat/_snaps/edition.md +++ b/tests/testthat/_snaps/edition.md @@ -1,3 +1,16 @@ +# checks its inputs + + Code + local_edition("x") + Condition + Error in `local_edition()`: + ! `x` must be a whole number, not the string "x". + Code + local_edition(5) + Condition + Error in `local_edition()`: + ! `x` must be a whole number between 2 and 3, not the number 5. + # deprecation only fired for newer edition Code diff --git a/tests/testthat/test-edition.R b/tests/testthat/test-edition.R index 628cda85e..9e3810fc2 100644 --- a/tests/testthat/test-edition.R +++ b/tests/testthat/test-edition.R @@ -6,15 +6,11 @@ test_that("can locally override edition", { expect_equal(edition_get(), 2) }) -test_that("only existing editions can be set", { - expect_error( - local_edition(5), - regexp = "Available editions are 2 and 3" - ) - expect_error( - local_edition(-1), - regexp = "Available editions are 2 and 3" - ) +test_that("checks its inputs", { + expect_snapshot(error = TRUE, { + local_edition("x") + local_edition(5) + }) }) test_that("deprecation only fired for newer edition", { @@ -45,7 +41,8 @@ test_that("edition for non-package dir is 2", { }) test_that("can set the edition via an environment variable", { - local_edition(zap()) + local_bindings(edition = zap(), .env = the) + withr::local_envvar(TESTTHAT_EDITION = 2) expect_equal(edition_get(), 2)