diff --git a/R/parallel.R b/R/parallel.R index cd07b4e10..7549fa5fd 100644 --- a/R/parallel.R +++ b/R/parallel.R @@ -83,7 +83,7 @@ test_files_parallel <- function( test_files_reporter_parallel <- function(reporter, .env = parent.frame()) { lister <- ListReporter$new() - snapshotter <- MainprocessSnapshotReporter$new("_snaps", fail_on_new = FALSE) + snapshotter <- MainprocessSnapshotReporter$new("_snaps") reporters <- list( find_reporter(reporter), lister, # track data @@ -301,10 +301,7 @@ queue_process_setup <- function( queue_task <- function(path) { withr::local_envvar("TESTTHAT_IS_PARALLEL" = "true") - snapshotter <- SubprocessSnapshotReporter$new( - snap_dir = "_snaps", - fail_on_new = FALSE - ) + snapshotter <- SubprocessSnapshotReporter$new(snap_dir = "_snaps") withr::local_options(testthat.snapshotter = snapshotter) reporters <- list( SubprocessReporter$new(), diff --git a/R/snapshot-file.R b/R/snapshot-file.R index 3dc32c2b3..8e6fbb99c 100644 --- a/R/snapshot-file.R +++ b/R/snapshot-file.R @@ -139,6 +139,10 @@ expect_snapshot_file <- function( variant = variant, trace_env = caller_env() ) + if (inherits(equal, "expectation_failure")) { + return(equal) + } + hint <- snapshot_review_hint(snapshotter$file, name) if (!equal) { @@ -196,7 +200,7 @@ snapshot_file_equal <- function( snap_variant, # variant (optional) path, # path to new file file_equal = compare_file_binary, - fail_on_new = FALSE, + fail_on_new = NULL, trace_env = caller_env() ) { if (!file.exists(path)) { @@ -208,6 +212,7 @@ snapshot_file_equal <- function( } else { snap_test_dir <- file.path(snap_dir, snap_variant, snap_test) } + fail_on_new <- fail_on_new %||% on_ci() cur_path <- file.path(snap_test_dir, snap_name) new_path <- file.path(snap_test_dir, new_name(snap_name)) @@ -232,12 +237,13 @@ snapshot_file_equal <- function( snap_name, "'" ) + + # We want to fail on CI since this suggests that the user has failed + # to record the value locally if (fail_on_new) { return(fail(message, trace_env = trace_env)) - } else { - testthat_warn(message) } - + testthat_warn(message) TRUE } } diff --git a/R/snapshot-reporter.R b/R/snapshot-reporter.R index c226518ff..c1105e6ff 100644 --- a/R/snapshot-reporter.R +++ b/R/snapshot-reporter.R @@ -8,13 +8,13 @@ SnapshotReporter <- R6::R6Class( test_file_seen = character(), snap_file_seen = character(), variants_changed = FALSE, - fail_on_new = FALSE, + fail_on_new = NULL, old_snaps = NULL, cur_snaps = NULL, new_snaps = NULL, - initialize = function(snap_dir = "_snaps", fail_on_new = FALSE) { + initialize = function(snap_dir = "_snaps", fail_on_new = NULL) { self$snap_dir <- normalizePath(snap_dir, mustWork = FALSE) self$fail_on_new <- fail_on_new }, @@ -82,19 +82,18 @@ SnapshotReporter <- R6::R6Class( value_enc <- save(value) self$cur_snaps$append(self$test, variant, value_enc) + fail_on_new <- self$fail_on_new %||% on_ci() message <- paste0( "Adding new snapshot", if (variant != "_default") paste0(" for variant '", variant, "'"), - if (self$fail_on_new) " in CI", ":\n", value_enc ) - if (self$fail_on_new) { + if (fail_on_new) { return(fail(message, trace_env = trace_env)) - } else { - testthat_warn(message) } + testthat_warn(message) character() } }, @@ -196,7 +195,7 @@ get_snapshotter <- function() { local_snapshotter <- function( snap_dir = NULL, cleanup = FALSE, - fail_on_new = FALSE, + fail_on_new = NULL, .env = parent.frame() ) { snap_dir <- snap_dir %||% withr::local_tempdir(.local_envir = .env) diff --git a/R/snapshot.R b/R/snapshot.R index 4e016c897..fb860c1e2 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -331,6 +331,9 @@ expect_snapshot_helper <- function( variant = variant, trace_env = trace_env ) + if (inherits(comp, "expectation_failure")) { + return(comp) + } if (!identical(variant, "_default")) { variant_lab <- paste0(" (variant '", variant, "')") diff --git a/R/test-files.R b/R/test-files.R index eb67f6fce..0995d7f4e 100644 --- a/R/test-files.R +++ b/R/test-files.R @@ -311,7 +311,7 @@ test_files_reporter <- function(reporter, .env = parent.frame()) { reporters <- list( find_reporter(reporter), lister, # track data - local_snapshotter("_snaps", fail_on_new = on_ci(), .env = .env) + local_snapshotter("_snaps", .env = .env) ) list( multi = MultiReporter$new(reporters = compact(reporters)), @@ -325,10 +325,18 @@ test_files_check <- function( stop_on_warning = FALSE ) { if (stop_on_failure && !all_passed(results)) { - cli::cli_abort("Test failures.") + cli::cli_abort( + "Test failures.", + call = NULL, + trace = data.frame() + ) } if (stop_on_warning && any_warnings(results)) { - cli::cli_abort("Tests generated warnings.") + cli::cli_abort( + "Tests generated warnings.", + call = NULL, + trace = data.frame() + ) } invisible(results) diff --git a/man/local_snapshotter.Rd b/man/local_snapshotter.Rd index 642c42869..bd473c3f4 100644 --- a/man/local_snapshotter.Rd +++ b/man/local_snapshotter.Rd @@ -7,7 +7,7 @@ local_snapshotter( snap_dir = NULL, cleanup = FALSE, - fail_on_new = FALSE, + fail_on_new = NULL, .env = parent.frame() ) } diff --git a/tests/testthat/_snaps/R4.6/snapshot-file/version.txt b/tests/testthat/_snaps/R4.6/snapshot-file/version.txt new file mode 100644 index 000000000..4f4edf2c1 --- /dev/null +++ b/tests/testthat/_snaps/R4.6/snapshot-file/version.txt @@ -0,0 +1 @@ +R4.6 diff --git a/tests/testthat/_snaps/R4.6/snapshot.md b/tests/testthat/_snaps/R4.6/snapshot.md new file mode 100644 index 000000000..bd7148cb2 --- /dev/null +++ b/tests/testthat/_snaps/R4.6/snapshot.md @@ -0,0 +1,7 @@ +# variants save different values + + Code + r_version() + Output + [1] "R4.6" + diff --git a/tests/testthat/_snaps/test-files.md b/tests/testthat/_snaps/test-files.md index 9f5c6fdc7..8b02cea1f 100644 --- a/tests/testthat/_snaps/test-files.md +++ b/tests/testthat/_snaps/test-files.md @@ -3,7 +3,7 @@ Code test_dir(test_path("test_dir"), reporter = "silent") Condition - Error in `test_files_check()`: + Error: ! Test failures. # runs all tests and records output @@ -40,7 +40,7 @@ Code test_error(stop_on_failure = TRUE) Condition - Error in `test_files_check()`: + Error: ! Test failures. # can control if warnings errors @@ -48,7 +48,7 @@ Code test_warning(stop_on_warning = TRUE) Condition - Error in `test_files_check()`: + Error: ! Tests generated warnings. # complains if file doesn't exist diff --git a/tests/testthat/test-parallel.R b/tests/testthat/test-parallel.R index 9e6bd1111..634529b32 100644 --- a/tests/testthat/test-parallel.R +++ b/tests/testthat/test-parallel.R @@ -60,8 +60,7 @@ test_that("fail", { test_that("snapshots", { withr::local_envvar(c(TESTTHAT_PARALLEL = "TRUE")) - withr::defer(unlink(tmp, recursive = TRUE)) - dir.create(tmp <- tempfile("testthat-snap-")) + tmp <- withr::local_tempdir("testthat-snap-") file.copy(test_path("test-parallel", "snap"), tmp, recursive = TRUE) # we cannot run these with the silent reporter, because it is not # parallel compatible, and they'll not run in parallel @@ -82,11 +81,11 @@ test_that("snapshots", { }) test_that("new snapshots are added", { - withr::local_envvar(c(TESTTHAT_PARALLEL = "TRUE")) - withr::defer(unlink(tmp, recursive = TRUE)) - dir.create(tmp <- tempfile("testthat-snap-")) + withr::local_envvar(c(TESTTHAT_PARALLEL = "TRUE", CI = "false")) + tmp <- withr::local_tempdir("testthat-snap-") file.copy(test_path("test-parallel", "snap"), tmp, recursive = TRUE) unlink(file.path(tmp, "snap", "tests", "testthat", "_snaps", "snap-2.md")) + # we cannot run these with the silent reporter, because it is not # parallel compatible, and they'll not run in parallel capture.output(suppressMessages( @@ -107,13 +106,13 @@ test_that("new snapshots are added", { test_that("snapshots are removed if test file has no snapshots", { withr::local_envvar(c(TESTTHAT_PARALLEL = "TRUE")) - withr::defer(unlink(tmp, recursive = TRUE)) - dir.create(tmp <- tempfile("testthat-snap-")) + tmp <- withr::local_tempdir("testthat-snap-") file.copy(test_path("test-parallel", "snap"), tmp, recursive = TRUE) writeLines( "test_that(\"2\", { expect_true(TRUE) })", file.path(tmp, "snap", "tests", "testthat", "test-snap-2.R") ) + # we cannot run these with the silent reporter, because it is not # parallel compatible, and they'll not run in parallel capture.output(suppressMessages( diff --git a/tests/testthat/test-snapshot-file.R b/tests/testthat/test-snapshot-file.R index a21b49775..fe5fcb459 100644 --- a/tests/testthat/test-snapshot-file.R +++ b/tests/testthat/test-snapshot-file.R @@ -86,7 +86,8 @@ test_that("warns on first creation", { snap_test = "my-test", snap_name = "test.txt", snap_variant = NULL, - path = path + path = path, + fail_on_new = FALSE ) } diff --git a/tests/testthat/test-snapshot-reporter.R b/tests/testthat/test-snapshot-reporter.R index 7f23f3118..c20f47ea0 100644 --- a/tests/testthat/test-snapshot-reporter.R +++ b/tests/testthat/test-snapshot-reporter.R @@ -38,6 +38,17 @@ test_that("basic workflow", { expect_true(file.exists(file.path(path, "snapshot-2.new.md"))) }) +test_that("defaults to failing on CI", { + withr::local_envvar(CI = "true") + + path <- withr::local_tempdir() + snapper <- local_snapshotter(path) + snapper$start_file("snapshot-2") + # warns on first creation + snapper$start_file("snapshot-2", "test") + expect_error(expect_snapshot_output("x"), "Adding new") +}) + test_that("only create new files for changed variants", { snapper <- local_snapshotter(fail_on_new = FALSE) snapper$start_file("variants", "test")