Skip to content

Commit

Permalink
Feedback from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
hadley committed Aug 1, 2023
1 parent d089352 commit 644ae77
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 40 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# testthat (development version)

* `test_file()` gains a `desc` argument which allows you to run a single
test from a file (#1776).

* `expect_setequal()` correctly displays results when only one of actual and
expected is missing values (#1835).

Expand Down
22 changes: 12 additions & 10 deletions R/source.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#' @param path Path to files.
#' @param pattern Regular expression used to filter files.
#' @param env Environment in which to evaluate code.
#' @param label If not-`NULL`, will run only test with this label.
#' @param desc If not-`NULL`, will run only test with this `desc`ription.
#' @param chdir Change working directory to `dirname(path)`?
#' @param wrap Automatically wrap all code within [test_that()]? This ensures
#' that all expectations are reported, even if outside a test block.
Expand All @@ -14,7 +14,7 @@
source_file <- function(path,
env = test_env(),
chdir = TRUE,
label = NULL,
desc = NULL,
wrap = TRUE,
error_call = caller_env()) {
stopifnot(file.exists(path))
Expand All @@ -28,7 +28,7 @@ source_file <- function(path,
con <- textConnection(lines, encoding = "UTF-8")
on.exit(try(close(con), silent = TRUE), add = TRUE)
exprs <- parse(con, n = -1, srcfile = srcfile, encoding = "UTF-8")
exprs <- filter_label(exprs, label, error_call = error_call)
exprs <- filter_desc(exprs, desc, error_call = error_call)

n <- length(exprs)
if (n == 0L) return(invisible())
Expand Down Expand Up @@ -60,8 +60,8 @@ source_file <- function(path,
}
}

filter_label <- function(exprs, label = NULL, error_call = caller_env()) {
if (is.null(label)) {
filter_desc <- function(exprs, desc = NULL, error_call = caller_env()) {
if (is.null(desc)) {
return(exprs)
}

Expand All @@ -72,25 +72,27 @@ filter_label <- function(exprs, label = NULL, error_call = caller_env()) {
expr <- exprs[[i]]

if (!is_call(expr, "test_that", n = 2)) {
include[[i]] <- TRUE
if (!found) {
include[[i]] <- TRUE
}
} else {
if (!is_string(expr[[2]]))
next

test_label <- as.character(expr[[2]])
if (test_label != label)
test_desc <- as.character(expr[[2]])
if (test_desc != desc)
next

if (found) {
abort("Found multiple tests with specified label", call = error_call)
abort("Found multiple tests with specified description", call = error_call)
}
include[[i]] <- TRUE
found <- TRUE
}
}

if (!found) {
abort("Failed to find test with specified label", call = error_call)
abort("Failed to find test with specified description", call = error_call)
}

exprs[include]
Expand Down
24 changes: 12 additions & 12 deletions R/test-files.R
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ test_dir <- function(path,
)
}

#' Run all tests in a single file
#' Run tests in a single file
#'
#' Helper, setup, and teardown files located in the same directory as the
#' test will also be run. See `vignette("special-files")` for details.
Expand All @@ -109,17 +109,17 @@ test_dir <- function(path,
#' @inheritSection test_dir Environments
#' @param path Path to file.
#' @param ... Additional parameters passed on to `test_dir()`
#' @param label Optionally, supply a string here to run only tests that
#' with this exact label.
#' @param desc Optionally, supply a string here to run only a single
#' tests that with this `desc`ription.
#' @export
#' @examples
#' path <- testthat_example("success")
#' test_file(path)
#' test_file(path, label = "some tests have warnings")
#' test_file(path, desc = "some tests have warnings")
#' test_file(path, reporter = "minimal")
test_file <- function(path,
reporter = default_compact_reporter(),
label = NULL,
desc = NULL,
package = NULL,
...) {
if (!file.exists(path)) {
Expand All @@ -131,7 +131,7 @@ test_file <- function(path,
test_package = package,
test_paths = basename(path),
reporter = reporter,
label = label,
desc = desc,
...
)
}
Expand All @@ -144,7 +144,7 @@ test_files <- function(test_dir,
env = NULL,
stop_on_failure = FALSE,
stop_on_warning = FALSE,
label = NULL,
desc = NULL,
wrap = TRUE,
load_package = c("none", "installed", "source"),
parallel = FALSE,
Expand Down Expand Up @@ -181,7 +181,7 @@ test_files <- function(test_dir,
env = env,
stop_on_failure = stop_on_failure,
stop_on_warning = stop_on_warning,
label = label,
desc = desc,
wrap = wrap,
load_package = load_package,
error_call = error_call
Expand All @@ -198,7 +198,7 @@ test_files_serial <- function(test_dir,
env = NULL,
stop_on_failure = FALSE,
stop_on_warning = FALSE,
label = NULL,
desc = NULL,
wrap = TRUE,
load_package = c("none", "installed", "source"),
error_call = caller_env()) {
Expand All @@ -215,7 +215,7 @@ test_files_serial <- function(test_dir,
test_paths,
test_one_file,
env = env,
label = label,
desc = desc,
wrap = wrap,
error_call = error_call
)
Expand Down Expand Up @@ -324,7 +324,7 @@ test_files_check <- function(results, stop_on_failure = TRUE, stop_on_warning =

test_one_file <- function(path,
env = test_env(),
label = NULL,
desc = NULL,
wrap = TRUE,
error_call = caller_env()) {
reporter <- get_reporter()
Expand All @@ -335,7 +335,7 @@ test_one_file <- function(path,
path,
env = env(env),
wrap = wrap,
label = label,
desc = desc,
error_call = error_call
)
reporter$end_context_if_started()
Expand Down
4 changes: 2 additions & 2 deletions man/source_file.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions man/test_file.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions tests/testthat/_snaps/source.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
# can find only matching test

Code
filter_label(code, "baz")
filter_desc(code, "baz")
Condition
Error:
! Failed to find test with specified label
! Failed to find test with specified description

# preserve srcrefs

Code
filter_label(code, "foo")
filter_desc(code, "foo")
Output
expression(test_that("foo", {
# this is a comment
Expand All @@ -28,8 +28,8 @@
# errors if duplicate labels

Code
filter_label(code, "baz")
filter_desc(code, "baz")
Condition
Error:
! Found multiple tests with specified label
! Found multiple tests with specified description

13 changes: 7 additions & 6 deletions tests/testthat/test-source.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ test_that("can find only matching test", {
code <- exprs(
f(),
test_that("foo", {}),
g(),
test_that("bar", {}),
g()
h()
)
expect_equal(filter_label(code, "foo"), code[c(1, 2, 4)])
expect_equal(filter_label(code, "bar"), code[c(1, 3, 4)])
expect_snapshot(filter_label(code, "baz"), error = TRUE)
expect_equal(filter_desc(code, "foo"), code[c(1, 2)])
expect_equal(filter_desc(code, "bar"), code[c(1, 3, 4)])
expect_snapshot(filter_desc(code, "baz"), error = TRUE)
})

test_that("preserve srcrefs", {
Expand All @@ -67,7 +68,7 @@ test_that("preserve srcrefs", {
# this is a comment
})
')
expect_snapshot(filter_label(code, "foo"))
expect_snapshot(filter_desc(code, "foo"))
})


Expand All @@ -79,5 +80,5 @@ test_that("errors if duplicate labels", {
g()
)

expect_snapshot(filter_label(code, "baz"), error = TRUE)
expect_snapshot(filter_desc(code, "baz"), error = TRUE)
})

0 comments on commit 644ae77

Please sign in to comment.