Swap more succeed()s with pass()es (#2146)

Author: hadley

R/expect-self-test.R

@@ -114,10 +114,8 @@ expect_no_success <- function(expr) {
 
   if (status$n_success > 0) {
     return(fail("Expectation succeeded"))
-  } else {
-    succeed()
   }
-  invisible(NULL)
+  pass(NULL)
 }
 
 #' @export
@@ -128,10 +126,8 @@ expect_no_failure <- function(expr) {
 
   if (status$n_failure > 0) {
     return(fail("Expectation failed"))
-  } else {
-    succeed()
   }
-  invisible(NULL)
+  pass(NULL)
 }
 
 expect_snapshot_skip <- function(x, cran = FALSE) {

tests/testthat/_snaps/expect-self-test.md

@@ -25,7 +25,7 @@
 # expect_no are deprecated
 
     Code
-      expect_no_failure(succeed())
+      expect_no_failure(pass(NULL))
     Condition
       Warning:
       `expect_no_failure()` was deprecated in testthat 3.3.0.

tests/testthat/reporters/tests.R

@@ -3,7 +3,7 @@ local_edition(2)
 context("Successes")
 
 test_that("Success", {
-  succeed()
+  pass(NULL)
 })
 
 context("Failures")

tests/testthat/test-expect-self-test.R

@@ -2,16 +2,16 @@ test_that("expect_failure() requires 1 failure and zero successes", {
   expect_success(expect_failure(fail()))
 
   expect_failure(expect_failure({}))
-  expect_failure(expect_failure(succeed()))
+  expect_failure(expect_failure(pass(NULL)))
   expect_failure(expect_failure({
-    succeed()
+    pass(NULL)
     fail()
   }))
 
   expect_failure(expect_failure({
     fail()
     # Following succeed/fail are never reached
-    succeed()
+    pass(NULL)
     fail()
   }))
 })
@@ -24,17 +24,17 @@ test_that("expect_failure() can optionally match message", {
 })
 
 test_that("expect_success() requires 1 success and zero failures", {
-  expect_success(expect_success(succeed()))
+  expect_success(expect_success(pass(NULL)))
 
   expect_failure(expect_success({}))
   expect_failure(expect_success(fail()))
   expect_failure(expect_success({
-    succeed()
+    pass(NULL)
     fail()
   }))
   expect_failure(expect_success({
-    succeed()
-    succeed()
+    pass(NULL)
+    pass(NULL)
   }))
 })
 
@@ -53,18 +53,18 @@ test_that("can count successes and failures", {
   expect_equal(status$n_failure, 0)
 
   status <- capture_success_failure({
-    succeed()
-    succeed()
+    pass(NULL)
+    pass(NULL)
     fail()
   })
   expect_equal(status$n_success, 2)
   expect_equal(status$n_failure, 1)
 
   # No code run after first fail
   status <- capture_success_failure({
-    succeed()
+    pass(NULL)
     fail()
-    succeed()
+    pass(NULL)
     fail()
   })
   expect_equal(status$n_success, 2)
@@ -73,15 +73,15 @@ test_that("can count successes and failures", {
 
 test_that("expect_no are deprecated", {
   expect_snapshot({
-    expect_no_failure(succeed())
+    expect_no_failure(pass(NULL))
     expect_no_success(fail())
   })
 })
 
 test_that("expect_no still work", {
   withr::local_options(lifecycle_verbosity = "quiet")
-  expect_success(expect_no_failure(succeed()))
+  expect_success(expect_no_failure(pass(NULL)))
   expect_failure(expect_no_failure(fail()))
   expect_success(expect_no_success(fail()))
-  expect_failure(expect_no_success(succeed()))
+  expect_failure(expect_no_success(pass(NULL)))
 })

tests/testthat/test-test-that.R

@@ -9,7 +9,7 @@ test_that("can't access variables from other tests (2)", {
 test_that("messages are suppressed", {
   local_edition(2)
   message("YOU SHOULDN'T SEE ME")
-  succeed()
+  pass(NULL)
 })
 
 test_that("errors are captured", {
@@ -72,7 +72,7 @@ test_that("return value from test_that", {
   with_reporter(
     "",
     success <- test_that("success", {
-      succeed()
+      pass(NULL)
     })
   )
   expect_true(success)

tests/testthat/test-try-again.R

@@ -2,10 +2,9 @@ succeed_after <- function(i) {
   function() {
     i <<- i - 1
     if (i > 0) {
-      fail(paste0("i is ", i))
-    } else {
-      succeed()
+      return(fail(paste0("i is ", i)))
     }
+    pass(NULL)
   }
 }