From 3f70084bcdb374c94d7e98ba30aa4545d0bbb415 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 5 Nov 2024 07:21:11 -0600
Subject: [PATCH] Don't emit success if test doesn't succeed (#2010)

Fixes #1997
---
 NEWS.md                                   |  1 +
 R/expect-no-condition.R                   |  6 ++++--
 tests/testthat/test-expect-no-condition.R | 17 +++++++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index ecc0b345d..64ae36713 100644
--- a/NEWS.md
+++ b/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).
diff --git a/R/expect-no-condition.R b/R/expect-no-condition.R
index f3a2318ca..ac79e922c 100644
--- a/R/expect-no-condition.R
+++ b/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)
 }
 
diff --git a/tests/testthat/test-expect-no-condition.R b/tests/testthat/test-expect-no-condition.R
index 059ba468d..f5c2c0d43 100644
--- a/tests/testthat/test-expect-no-condition.R
+++ b/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")