Skip to content

Commit

Permalink
Make interactive snapshot output a bit more obvious
Browse files Browse the repository at this point in the history
Testing this change revealed a bug in `test_code()`: specifying the default reporter happened _after_ `local_test_context()` meaning that it failed to capture the actual user settings. I fixed this by now requiring a reporter (rather than a backup reporter).

Fixes #1992
  • Loading branch information
hadley committed Oct 29, 2024
1 parent 75c70e0 commit 2b36f1b
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 12 deletions.
3 changes: 2 additions & 1 deletion R/describe.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,14 @@ describe <- function(description, code) {
}

describe_it <- function(description, code, env = parent.frame()) {
reporter <- get_reporter() %||% local_interactive_reporter()
local_test_context()

test_code(
description,
code,
env = env,
default_reporter = local_interactive_reporter(),
reporter = reporter,
skip_on_empty = FALSE
)
}
Expand Down
2 changes: 1 addition & 1 deletion R/snapshot-file.R
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ expect_snapshot_file <- function(path,

snapshotter <- get_snapshotter()
if (is.null(snapshotter)) {
snapshot_not_available(paste0("New path: ", path))
snapshot_not_available("New path", path)
return(invisible())
}

Expand Down
12 changes: 7 additions & 5 deletions R/snapshot.R
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ expect_snapshot_helper <- function(lab, val,

snapshotter <- get_snapshotter()
if (is.null(snapshotter)) {
snapshot_not_available(paste0("Current value:\n", save(val)))
snapshot_not_available("Current snapshot", save(val))
return(invisible())
}

Expand Down Expand Up @@ -322,12 +322,15 @@ snapshot_accept_hint <- function(variant, file, reset_output = TRUE) {
)
}

snapshot_not_available <- function(message) {
snapshot_not_available <- function(header, message) {
local_reporter_output()

cli::cli_inform(c(
"{.strong Can't compare snapshot to reference when testing interactively.}",
i = "Run {.run devtools::test()} or {.code testthat::test_file()} to see changes."
i = "Can't create snapshot or compare to reference when testing interactively."
))
cat(cli::rule(header), "\n", sep = "")
cat(message, "\n", sep = "")
cat(cli::rule(), "\n", sep = "")
}

local_snapshot_dir <- function(snap_names, .env = parent.frame()) {
Expand Down Expand Up @@ -362,4 +365,3 @@ with_is_snapshotting <- function(code) {
withr::local_envvar(TESTTHAT_IS_SNAPSHOT = "true")
code
}

2 changes: 1 addition & 1 deletion R/source.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ source_file <- function(path,
test = NULL,
code = exprs,
env = env,
default_reporter = StopReporter$new()
reporter = get_reporter() %||% StopReporter$new()
))
} else {
withCallingHandlers(
Expand Down
2 changes: 1 addition & 1 deletion R/test-example.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ test_example <- function(path, title = path) {
test = title,
code = parse(ex_path, encoding = "UTF-8"),
env = env,
default_reporter = StopReporter$new(),
reporter = get_reporter() %||% StopReporter$new(),
skip_on_empty = FALSE
)
if (ok) succeed(path)
Expand Down
8 changes: 5 additions & 3 deletions R/test-that.R
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,23 @@ test_that <- function(desc, code) {
}
}

# Must initialise interactive reporter before local_test_context()
reporter <- get_reporter() %||% local_interactive_reporter()
local_test_context()

test_code(
desc,
code,
env = parent.frame(),
default_reporter = local_interactive_reporter()
reporter = reporter
)
}

# Access error fields with `[[` rather than `$` because the
# `$.Throwable` from the rJava package throws with unknown fields
test_code <- function(test, code, env, default_reporter, skip_on_empty = TRUE) {
test_code <- function(test, code, env, reporter, skip_on_empty = TRUE) {

frame <- caller_env()
reporter <- get_reporter() %||% default_reporter

if (!is.null(test)) {
reporter$start_test(context = reporter$.context, test = test)
Expand Down

0 comments on commit 2b36f1b

Please sign in to comment.