Don't emit success if test doesn't succeed (#2010)

Fixes #1997

NEWS.md

@@ -1,5 +1,6 @@
 # testthat (development version)
 
+* `expect_no_*()` expectations no longer incorrectly emit a passing test result if they in fact fail (#1997).
 * Require the latest version of waldo (0.6.0) in order to get the latest goodies (#1955).
 * `expect_visible()` and `expect_invisible()` have improved failure messages (#1966).
 * `expect_snapshot()` now strips line breaks in test descriptions (@LDSamson, #1900).

R/expect-no-condition.R

@@ -96,7 +96,10 @@ expect_no_ <- function(base_class,
 
   capture <- function(code) {
     try_fetch(
-      code,
+      {
+        code
+        succeed()
+      },
       !!base_class := function(cnd) {
         if (!matcher(cnd)) {
           return(zap())
@@ -119,7 +122,6 @@ expect_no_ <- function(base_class,
   }
 
   act <- quasi_capture(enquo(object), NULL, capture)
-  succeed()
   invisible(act$val)
 }
 

tests/testthat/test-expect-no-condition.R

@@ -13,6 +13,23 @@ test_that("expect_no_* conditions behave as expected", {
 
 })
 
+test_that("expect_no_* don't emit success when they fail", {
+
+  catch_cnds <- function(code) {
+    cnds <- list()
+
+    withCallingHandlers(code, condition = function(cnd) {
+      cnds[[length(cnds) + 1]] <<- cnd
+      invokeRestart("continue_test")
+    })
+    cnds
+  }
+
+  cnds <- catch_cnds(expect_no_error(stop("!")))
+  expect_length(cnds, 1)
+  expect_s3_class(cnds[[1]], "expectation_failure")
+})
+
 test_that("unmatched conditions bubble up", {
   expect_error(expect_no_error(abort("foo"), message = "bar"), "foo")
   expect_warning(expect_no_warning(warn("foo"), message = "bar"), "foo")