Ensure interactive tests error appropriately (#1443)

Fixes #1430

Author: hadley

NEWS.md

@@ -1,5 +1,8 @@
 # testthat (development version)
 
+* When run interactively, `test_that()` now correctly errors when an expectation
+  fails (#1430).
+
 * Uncaught errors now show their class (#1426).
 
 * `local_reproducible_output()` now sets the `max.print` option to 99999 

R/local.R

@@ -75,13 +75,12 @@ local_test_context <- function(.env = parent.frame()) {
 #'   The test is skipped if `` l10n_info()$`UTF-8` `` is `FALSE`.
 #' @rdname local_test_context
 #' @examples
-#'
-#' ellipsis <- cli::symbol$ellipsis
 #' test_that("test ellipsis", {
-#'   expect_equal(ellipsis, cli::symbol$ellipsis)
+#'   local_reproducible_output(unicode = FALSE)
+#'   expect_equal(cli::symbol$ellipsis, "...")
 #'
 #'   local_reproducible_output(unicode = TRUE)
-#'   expect_equal(ellipsis, cli::symbol$ellipsis)
+#'   expect_equal(cli::symbol$ellipsis, "\u2026")
 #' })
 local_reproducible_output <- function(width = 80,
                                       crayon = FALSE,
@@ -163,8 +162,9 @@ local_interactive_reporter <- function(.env = parent.frame()) {
   local_edition(find_edition("."), .env = .env)
 
   # Use StopReporter
-  reporter <- StopReporter$new(stop_reporter = FALSE)
+  reporter <- StopReporter$new()
   old <- set_reporter(reporter)
+  withr::defer(reporter$stop_if_needed(), envir = .env)
   withr::defer(set_reporter(old), envir = .env)
 
   reporter

R/reporter-stop.R

@@ -1,6 +1,6 @@
-#' Test reporter: stop on error.
+#' Test reporter: stop on error
 #'
-#' The default reporter, executed when `expect_that` is run interactively.
+#' The default reporter used when [expect_that()] is run interactively.
 #' It responds by [stop()]ping on failures and doing nothing otherwise. This
 #' will ensure that a failing test will raise an error.
 #'
@@ -50,9 +50,10 @@ StopReporter <- R6::R6Class("StopReporter",
 
       messages <- vapply(failures, issue_summary, rule = TRUE, character(1))
       self$cat_line(messages, "\n")
-
-      if (self$stop_reporter && self$n_fail > 1) {
-        stop_reporter("Test failed")
+    },
+    stop_if_needed = function() {
+      if (self$stop_reporter && self$n_fail > 0) {
+        abort("Test failed")
       }
     }
   )

R/test-that.R

@@ -1,27 +1,27 @@
-#' Create a test.
+#' Run a test
 #'
-#' A test encapsulates a series of expectations about small, self-contained
-#' set of functionality. Each test is contained in a \link{context} and
-#' contains multiple expectations.
+#' @description
+#' A test encapsulates a series of expectations about a small, self-contained
+#' set of functionality. Each test lives in a file and contains multiple
+#' expectations, like [expect_equal()] or [expect_error()].
 #'
 #' Tests are evaluated in their own environments, and should not affect
 #' global state.
 #'
-#' When run from the command line, tests return `NULL` if all
-#' expectations are met, otherwise it raises an error.
-#'
-#' @param desc test name.  Names should be kept as brief as possible, as they
-#'   are often used as line prefixes.
-#' @param code test code containing expectations.  Braces (`{}`) should always be
-#'   used in order to get accurate location data for test failures.
+#' @param desc Test name. Names should be brief, but evocative. They are
+#'   only used by humans, so do you
+#' @param code Test code containing expectations. Braces (`{}`) should always
+#'   be used in order to get accurate location data for test failures.
+#' @return When run interactively, returns `invisible(TRUE)` if all tests
+#'   pass, otherwise throws an error.
 #' @export
 #' @examples
 #' test_that("trigonometric functions match identities", {
 #'   expect_equal(sin(pi / 4), 1 / sqrt(2))
 #'   expect_equal(cos(pi / 4), 1 / sqrt(2))
 #'   expect_equal(tan(pi / 4), 1)
 #' })
-#' # Failing test:
+#'
 #' \dontrun{
 #' test_that("trigonometric functions match identities", {
 #'   expect_equal(sin(pi / 4), 1)

man/StopReporter.Rd

@@ -41,19 +41,3 @@
     Reason: empty test
 
 
-# stop stops at first failure
-
-    -- Failure (fail.R:4:3): two failures ------------------------------------------
-    FALSE is not TRUE
-    
-    `actual`:   FALSE
-    `expected`: TRUE 
-    
-    -- Failure (fail.R:5:3): two failures ------------------------------------------
-    TRUE is not FALSE
-    
-    `actual`:   TRUE 
-    `expected`: FALSE
-    
-    Test failed 
-

man/local_test_context.Rd

@@ -1,7 +1,12 @@
 test_that("produces useful output", {
-  expect_snapshot_reporter(StopReporter$new(stop_reporter = FALSE))
+  expect_snapshot_reporter(StopReporter$new())
 })
 
-test_that("stop stops at first failure", {
-  expect_snapshot_reporter(StopReporter$new(), test_path("reporters/fail.R"))
+test_that("stop if needed errors when needed",{
+  r <- StopReporter$new()
+  expect_error(r$stop_if_needed(), NA)
+  r$n_fail <- 1
+  expect_error(r$stop_if_needed(), "Test failed")
+  r$stop_reporter <- FALSE
+  expect_error(r$stop_if_needed(), NA)
 })

man/test_that.Rd

@@ -112,7 +112,7 @@ snapper$start_file("snapshotting.Rmd", "test")
 
 But if you change the underlying code, say to tweak the indenting, the test will fail:
 
-```{r}
+```{r, error = TRUE}
 bullets <- function(text, id = NULL) {
   paste0(
     "<ul", if (!is.null(id)) paste0(" id=\"", id, "\""), ">\n", 
@@ -138,7 +138,7 @@ Reviewers often don't run your code but still want to understand the changes.
 
 Here's the snapshot file generated by the test above:
 
-``` {.md}
+``` md
 # bullets
 
     <ul>
@@ -184,7 +184,7 @@ test_that("f() makes lots of noice", {
 
 Capturing errors is *slightly* more difficult because `expect_snapshot()` will fail when there's an error:
 
-```{r}
+```{r, error = TRUE}
 test_that("you can't add a number and a letter", {
   expect_snapshot(1 + "a")
 })