From 96e78624ab370366a314575dc003d8cf945033de Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 09:33:13 -0500 Subject: [PATCH 1/7] Extract a reprex from a failing expectation --- NAMESPACE | 1 + NEWS.md | 1 + R/extract.R | 100 ++++++++++++++++++++++++++++++++++++++++++++ man/extract_test.Rd | 19 +++++++++ 4 files changed, 121 insertions(+) create mode 100644 R/extract.R create mode 100644 man/extract_test.Rd diff --git a/NAMESPACE b/NAMESPACE index 426dbf1b4..3ebfd4fa2 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -139,6 +139,7 @@ export(expect_vector) export(expect_visible) export(expect_warning) export(expectation) +export(extract_test) export(fail) export(find_test_scripts) export(get_reporter) diff --git a/NEWS.md b/NEWS.md index 207716bf1..25939e493 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* New `extract_test()` function to extract a reprex from a failing expectation. * Expectations now consistently return the value of the first argument, regardless of whether the expectation succeeds or fails. The primary exception are `expect_message()` and friends which will return the condition. This shouldn't affect existing tests, but will make failures clearer when you chain together multiple expectations (#2246). * `set_state_inspector()` gains `tolerance` argument and ignores minor FP differences by default (@mcol, #2237). * `expect_vector()` fails, instead of erroring, if `object` is not a vector (@plietar, #2224). diff --git a/R/extract.R b/R/extract.R new file mode 100644 index 000000000..a2490c951 --- /dev/null +++ b/R/extract.R @@ -0,0 +1,100 @@ +#' Extract a reprex from an failed expectation +#' +#' `extract_test()` creates a minimal reprex for a failed expectation. +#' It extracts all non-test code before the failed expectation as well as +#' all code inside the test up to and including the failed expectation. +#' +#' @param location A string giving the location in the form +#' `FILE:LINE[:COLUMN]`. +#' @param path Path to write the reprex to. Defaults to `stdout()`. +#' @export +extract_test <- function(location, path = stdout()) { + check_string(location) + + pieces <- strsplit(location, ":")[[1]] + if (!length(pieces) %in% c(2, 3)) { + cli::cli_abort( + "Expected {.arg location} to be of the form FILE:LINE[:COLUMN]" + ) + } + + test_path <- test_path(pieces[[1]]) + line <- as.integer(pieces[2]) + + lines <- extract_test_lines(test_path, line) + base::writeLines(lines, con = path) +} + +extract_test_lines <- function(path, line, error_call = caller_env()) { + check_string(path) + if (!file.exists(path)) { + cli::cli_abort( + "{.arg path} ({.path path}) does not exist.", + call = error_call + ) + } + check_number_whole(line, min = 1, call = error_call) + + exprs <- parse(file = path, keep.source = TRUE) + srcrefs <- attr(exprs, "srcref") + + # Focus on srcrefs before the selected line + keep <- start_line(srcrefs) <= line + exprs <- exprs[keep] + srcrefs <- srcrefs[keep] + + # We first capture the prequel, all code outside of tests + is_subtest <- map_lgl(exprs, is_subtest) + if (any(!is_subtest)) { + prequel <- c( + comment_header("prequel"), + map_chr(srcrefs[!is_subtest], as.character), + "" + ) + } else { + prequel <- NULL + } + + # Now we extract the contents of the test + test_idx <- rev(which(is_subtest))[[1]] + call <- exprs[[test_idx]] + check_test_call(call, error_call = error_call) + + test_contents <- attr(call[[3]], "srcref")[-1] # drop `{` + keep <- start_line(test_contents) <= line + test <- map_chr(test_contents[keep], as.character) + + c( + paste0("# Extracted from tests/testthat/", path, ":", line), + prequel, + comment_header("test"), + test + ) +} + +# Helpers --------------------------------------------------------------------- + +check_test_call <- function(expr, error_call = caller_env()) { + if (!is_call(expr, n = 2)) { + cli::cli_abort( + "test call has unexpected number of arguments", + internal = TRUE, + call = error_call + ) + } + if (!is_call(expr[[3]], "{")) { + cli::cli_abort( + "test call has use {", + internal = TRUE, + call = error_call + ) + } +} + +comment_header <- function(x) { + paste0("# ", x, " ", strrep("-", 80 - nchar(x) - 3)) +} + +start_line <- function(srcrefs) { + map_int(srcrefs, \(x) x[[1]]) +} diff --git a/man/extract_test.Rd b/man/extract_test.Rd new file mode 100644 index 000000000..914fa7897 --- /dev/null +++ b/man/extract_test.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/extract.R +\name{extract_test} +\alias{extract_test} +\title{Extract a reprex from an failed expectation} +\usage{ +extract_test(location, path = stdout()) +} +\arguments{ +\item{location}{A string giving the location in the form +\verb{FILE:LINE[:COLUMN]}.} + +\item{path}{Path to write the reprex to. Defaults to \code{stdout()}.} +} +\description{ +\code{extract_test()} creates a minimal reprex for a failed expectation. +It extracts all non-test code before the failed expectation as well as +all code inside the test up to and including the failed expectation. +} From 4e5b189e37fe3c23ccf1a305a4c6fa34ab323da8 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 09:40:16 -0500 Subject: [PATCH 2/7] Ensure it never errors so could be automated --- R/extract.R | 25 ++++++++++++++++++------- man/extract_test.Rd | 5 +++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/R/extract.R b/R/extract.R index a2490c951..dd33ba6b3 100644 --- a/R/extract.R +++ b/R/extract.R @@ -7,6 +7,9 @@ #' @param location A string giving the location in the form #' `FILE:LINE[:COLUMN]`. #' @param path Path to write the reprex to. Defaults to `stdout()`. +#' @return This function is called for its side effect of rendering a +#' reprex to `path`. This function will never error: if extraction +#' fails, the error message will be written to `path`. #' @export extract_test <- function(location, path = stdout()) { check_string(location) @@ -20,8 +23,18 @@ extract_test <- function(location, path = stdout()) { test_path <- test_path(pieces[[1]]) line <- as.integer(pieces[2]) + source <- paste0("# Extracted from tests/testthat/", path, ":", line) + + lines <- tryCatch( + extract_test_lines(test_path, line), + error = function(cnd) { + lines <- strsplit(conditionMessage(cnd), "\n")[[1]] + lines <- c("Failed to extract test", lines) + paste0("# ", lines) + } + ) + lines <- c(source, lines) - lines <- extract_test_lines(test_path, line) base::writeLines(lines, con = path) } @@ -62,14 +75,12 @@ extract_test_lines <- function(path, line, error_call = caller_env()) { test_contents <- attr(call[[3]], "srcref")[-1] # drop `{` keep <- start_line(test_contents) <= line - test <- map_chr(test_contents[keep], as.character) - - c( - paste0("# Extracted from tests/testthat/", path, ":", line), - prequel, + test <- c( comment_header("test"), - test + map_chr(test_contents[keep], as.character) ) + + c(prequel, test) } # Helpers --------------------------------------------------------------------- diff --git a/man/extract_test.Rd b/man/extract_test.Rd index 914fa7897..ce8f9b7b3 100644 --- a/man/extract_test.Rd +++ b/man/extract_test.Rd @@ -12,6 +12,11 @@ extract_test(location, path = stdout()) \item{path}{Path to write the reprex to. Defaults to \code{stdout()}.} } +\value{ +This function is called for its side effect of rendering a +reprex to \code{path}. This function will never error: if extraction +fails, the error message will be written to \code{path}. +} \description{ \code{extract_test()} creates a minimal reprex for a failed expectation. It extracts all non-test code before the failed expectation as well as From 20a533f6c1e6b9357a1ea23e60abfad488dbca69 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 12:17:06 -0500 Subject: [PATCH 3/7] Claude feedback --- R/extract.R | 13 ++++++++----- man/extract_test.Rd | 2 +- tests/testthat/test-expect-output.R | 2 ++ 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/R/extract.R b/R/extract.R index dd33ba6b3..875491a1b 100644 --- a/R/extract.R +++ b/R/extract.R @@ -1,4 +1,4 @@ -#' Extract a reprex from an failed expectation +#' Extract a reprex from a failed expectation #' #' `extract_test()` creates a minimal reprex for a failed expectation. #' It extracts all non-test code before the failed expectation as well as @@ -23,13 +23,13 @@ extract_test <- function(location, path = stdout()) { test_path <- test_path(pieces[[1]]) line <- as.integer(pieces[2]) - source <- paste0("# Extracted from tests/testthat/", path, ":", line) + source <- paste0("# Extracted from ", test_path, ":", line) lines <- tryCatch( extract_test_lines(test_path, line), error = function(cnd) { lines <- strsplit(conditionMessage(cnd), "\n")[[1]] - lines <- c("Failed to extract test", lines) + lines <- c("", "Failed to extract test: ", lines) paste0("# ", lines) } ) @@ -68,7 +68,10 @@ extract_test_lines <- function(path, line, error_call = caller_env()) { prequel <- NULL } - # Now we extract the contents of the test + # Now we extract the contents of the last test + if (!any(is_subtest)) { + cli::cli_abort("Failed to find test at line {line}.", call = error_call) + } test_idx <- rev(which(is_subtest))[[1]] call <- exprs[[test_idx]] check_test_call(call, error_call = error_call) @@ -95,7 +98,7 @@ check_test_call <- function(expr, error_call = caller_env()) { } if (!is_call(expr[[3]], "{")) { cli::cli_abort( - "test call has use {", + "test call doesn't use `{`", internal = TRUE, call = error_call ) diff --git a/man/extract_test.Rd b/man/extract_test.Rd index ce8f9b7b3..8d237388b 100644 --- a/man/extract_test.Rd +++ b/man/extract_test.Rd @@ -2,7 +2,7 @@ % Please edit documentation in R/extract.R \name{extract_test} \alias{extract_test} -\title{Extract a reprex from an failed expectation} +\title{Extract a reprex from a failed expectation} \usage{ extract_test(location, path = stdout()) } diff --git a/tests/testthat/test-expect-output.R b/tests/testthat/test-expect-output.R index 52e2adbd2..d8e0034b0 100644 --- a/tests/testthat/test-expect-output.R +++ b/tests/testthat/test-expect-output.R @@ -1,6 +1,8 @@ f <- function() NULL g <- function() cat("!") +writeLines("Hi!", "../someoutput.txt") + test_that("expect = NA checks for no output", { expect_success(expect_output(f(), NA)) expect_snapshot_failure(expect_output(g(), NA)) From 141ad83cd5bdc05eef9cfd696c3b32619b9d3de7 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 12:19:26 -0500 Subject: [PATCH 4/7] Refactor to simplify testing --- R/extract.R | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/R/extract.R b/R/extract.R index 875491a1b..a2583cb87 100644 --- a/R/extract.R +++ b/R/extract.R @@ -24,9 +24,10 @@ extract_test <- function(location, path = stdout()) { test_path <- test_path(pieces[[1]]) line <- as.integer(pieces[2]) source <- paste0("# Extracted from ", test_path, ":", line) + exprs <- parse_file(test_path) lines <- tryCatch( - extract_test_lines(test_path, line), + extract_test_lines(exprs, line), error = function(cnd) { lines <- strsplit(conditionMessage(cnd), "\n")[[1]] lines <- c("", "Failed to extract test: ", lines) @@ -38,17 +39,9 @@ extract_test <- function(location, path = stdout()) { base::writeLines(lines, con = path) } -extract_test_lines <- function(path, line, error_call = caller_env()) { - check_string(path) - if (!file.exists(path)) { - cli::cli_abort( - "{.arg path} ({.path path}) does not exist.", - call = error_call - ) - } +extract_test_lines <- function(exprs, line, error_call = caller_env()) { check_number_whole(line, min = 1, call = error_call) - exprs <- parse(file = path, keep.source = TRUE) srcrefs <- attr(exprs, "srcref") # Focus on srcrefs before the selected line @@ -88,6 +81,17 @@ extract_test_lines <- function(path, line, error_call = caller_env()) { # Helpers --------------------------------------------------------------------- +parse_file <- function(path, error_call = caller_env()) { + check_string(path, call = error_call) + if (!file.exists(path)) { + cli::cli_abort( + "{.arg path} ({.path path}) does not exist.", + call = error_call + ) + } + parse(path, keep.source = TRUE) +} + check_test_call <- function(expr, error_call = caller_env()) { if (!is_call(expr, n = 2)) { cli::cli_abort( From bc9fa1f34cb05f20d70111a5a36f99ff27e9f289 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 13:31:06 -0500 Subject: [PATCH 5/7] Add some basic tests & refine implementation --- R/extract.R | 76 +++++++++++++------------------- R/source.R | 4 +- tests/testthat/_snaps/extract.md | 30 +++++++++++++ tests/testthat/test-extract.R | 46 +++++++++++++++++++ 4 files changed, 110 insertions(+), 46 deletions(-) create mode 100644 tests/testthat/_snaps/extract.md create mode 100644 tests/testthat/test-extract.R diff --git a/R/extract.R b/R/extract.R index a2583cb87..d9f7d8eed 100644 --- a/R/extract.R +++ b/R/extract.R @@ -43,40 +43,33 @@ extract_test_lines <- function(exprs, line, error_call = caller_env()) { check_number_whole(line, min = 1, call = error_call) srcrefs <- attr(exprs, "srcref") - - # Focus on srcrefs before the selected line - keep <- start_line(srcrefs) <= line - exprs <- exprs[keep] - srcrefs <- srcrefs[keep] - - # We first capture the prequel, all code outside of tests is_subtest <- map_lgl(exprs, is_subtest) - if (any(!is_subtest)) { - prequel <- c( - comment_header("prequel"), - map_chr(srcrefs[!is_subtest], as.character), - "" - ) - } else { - prequel <- NULL - } - # Now we extract the contents of the last test - if (!any(is_subtest)) { + # First we find the test + is_test <- is_subtest & + start_line(srcrefs) <= line & + end_line(srcrefs) >= line + if (!any(is_test)) { cli::cli_abort("Failed to find test at line {line}.", call = error_call) } - test_idx <- rev(which(is_subtest))[[1]] - call <- exprs[[test_idx]] - check_test_call(call, error_call = error_call) - + call <- exprs[[which(is_test)[[1]]]] test_contents <- attr(call[[3]], "srcref")[-1] # drop `{` keep <- start_line(test_contents) <= line - test <- c( - comment_header("test"), - map_chr(test_contents[keep], as.character) - ) + test <- srcref_to_character(test_contents[keep]) + + # We first find the prequel, all non-test code before the test + is_prequel <- !is_subtest & start_line(srcrefs) < line + if (!any(is_prequel)) { + return(test) + } - c(prequel, test) + c( + "# prequel ---------------------------------------------------------------", + srcref_to_character(srcrefs[is_prequel]), + "", + "# test ------------------------------------------------------------------", + test + ) } # Helpers --------------------------------------------------------------------- @@ -92,27 +85,20 @@ parse_file <- function(path, error_call = caller_env()) { parse(path, keep.source = TRUE) } -check_test_call <- function(expr, error_call = caller_env()) { - if (!is_call(expr, n = 2)) { - cli::cli_abort( - "test call has unexpected number of arguments", - internal = TRUE, - call = error_call - ) - } - if (!is_call(expr[[3]], "{")) { - cli::cli_abort( - "test call doesn't use `{`", - internal = TRUE, - call = error_call - ) - } -} +parse_text <- function(text) { + text <- sub("^\n", "", text) + indent <- regmatches(text, regexpr("^ *", text)) + text <- gsub(paste0("(?m)^", indent), "", text, perl = TRUE) -comment_header <- function(x) { - paste0("# ", x, " ", strrep("-", 80 - nchar(x) - 3)) + parse(text = text, keep.source = TRUE) } +srcref_to_character <- function(x) { + unlist(map(x, as.character)) +} start_line <- function(srcrefs) { map_int(srcrefs, \(x) x[[1]]) } +end_line <- function(srcrefs) { + map_int(srcrefs, \(x) x[[3]]) +} diff --git a/R/source.R b/R/source.R index 3c069d9f9..e80bbd413 100644 --- a/R/source.R +++ b/R/source.R @@ -115,7 +115,9 @@ filter_desc <- function(exprs, descs, error_call = caller_env()) { } is_subtest <- function(expr) { - is_call(expr, c("test_that", "describe", "it"), n = 2) && is_string(expr[[2]]) + is_call(expr, c("test_that", "describe", "it"), n = 2) && + is_string(expr[[2]]) && + is_call(expr[[3]], "{") } #' @rdname source_file diff --git a/tests/testthat/_snaps/extract.md b/tests/testthat/_snaps/extract.md new file mode 100644 index 000000000..044b4fbe5 --- /dev/null +++ b/tests/testthat/_snaps/extract.md @@ -0,0 +1,30 @@ +# can extract prequel + + Code + base::writeLines(extract_test_lines(exprs, 4)) + Output + # prequel --------------------------------------------------------------- + x <- 1 + y <- 2 + + # test ------------------------------------------------------------------ + expect_true(TRUE) + +# preserves code format but not comments + + Code + base::writeLines(extract_test_lines(exprs, 3)) + Output + # prequel --------------------------------------------------------------- + 1 + 1 + + # test ------------------------------------------------------------------ + 2 + 2 + +# can extract selected expectation + + Code + base::writeLines(extract_test_lines(exprs, 2)) + Output + expect_true(TRUE) + diff --git a/tests/testthat/test-extract.R b/tests/testthat/test-extract.R new file mode 100644 index 000000000..36716a608 --- /dev/null +++ b/tests/testthat/test-extract.R @@ -0,0 +1,46 @@ +test_that("can extract prequel", { + # fmt: skip + exprs <- parse_text(" + x <- 1 + y <- 2 + test_that('foo', { + expect_true(TRUE) + }) + ") + expect_snapshot(base::writeLines(extract_test_lines(exprs, 4))) +}) + +test_that("preserves code format but not comments", { + # fmt: skip + exprs <- parse_text(" + 1 + 1 # 2 + test_that('foo', { + 2 + 2 # 4 + }) + ") + expect_snapshot(base::writeLines(extract_test_lines(exprs, 3))) +}) + +test_that("can extract selected expectation", { + # fmt: skip + exprs <- parse_text(" + test_that('foo', { + expect_true(TRUE) + expect_false(FALSE) + }) + ") + expect_snapshot(base::writeLines(extract_test_lines(exprs, 2))) +}) + +test_that("errors if can't find test", { + # fmt: skip + exprs <- parse_text(" + # line 1 + test_that('foo', { + expect_true(TRUE) + }) + # line 5 + ") + expect_error(extract_test_lines(exprs, 1), "Failed to find test") + expect_error(extract_test_lines(exprs, 5), "Failed to find test") +}) From d78f8aa699eb114db74a1967d4b14ccbb4d9b1fd Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 14:38:20 -0500 Subject: [PATCH 6/7] Add to reference index --- _pkgdown.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/_pkgdown.yml b/_pkgdown.yml index 0b9aa2f12..fbe3de2c2 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -54,6 +54,7 @@ reference: - title: Test helpers contents: - is_testing + - extract_test - local_edition - local_reproducible_output - set_state_inspector From 23d8276238462b69de1ec61eac77e034f91bb7dd Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Wed, 8 Oct 2025 14:40:54 -0500 Subject: [PATCH 7/7] Add example --- R/extract.R | 15 +++++++++++++++ man/extract_test.Rd | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/R/extract.R b/R/extract.R index d9f7d8eed..c0533fd71 100644 --- a/R/extract.R +++ b/R/extract.R @@ -1,9 +1,13 @@ #' Extract a reprex from a failed expectation #' +#' @description #' `extract_test()` creates a minimal reprex for a failed expectation. #' It extracts all non-test code before the failed expectation as well as #' all code inside the test up to and including the failed expectation. #' +#' This is particularly useful when you're debugging test failures in +#' someone else's package. +#' #' @param location A string giving the location in the form #' `FILE:LINE[:COLUMN]`. #' @param path Path to write the reprex to. Defaults to `stdout()`. @@ -11,6 +15,17 @@ #' reprex to `path`. This function will never error: if extraction #' fails, the error message will be written to `path`. #' @export +#' @examples +#' # If you see a test failure like this: +#' # ── Failure (test-extract.R:46:3): errors if can't find test ─────────────── +#' # Expected FALSE to be TRUE. +#' # Differences: +#' # `actual`: FALSE +#' # `expected`: TRUE +#' +#' # You can run this: +#' \dontrun{extract_test("test-extract.R:46:3")} +#' # to see just the code needed to reproduce the failure extract_test <- function(location, path = stdout()) { check_string(location) diff --git a/man/extract_test.Rd b/man/extract_test.Rd index 8d237388b..30d4aa585 100644 --- a/man/extract_test.Rd +++ b/man/extract_test.Rd @@ -21,4 +21,19 @@ fails, the error message will be written to \code{path}. \code{extract_test()} creates a minimal reprex for a failed expectation. It extracts all non-test code before the failed expectation as well as all code inside the test up to and including the failed expectation. + +This is particularly useful when you're debugging test failures in +someone else's package. +} +\examples{ +# If you see a test failure like this: +# ── Failure (test-extract.R:46:3): errors if can't find test ─────────────── +# Expected FALSE to be TRUE. +# Differences: +# `actual`: FALSE +# `expected`: TRUE + +# You can run this: +\dontrun{extract_test("test-extract.R:46:3")} +# to see just the code needed to reproduce the failure }