From 40573f0b8eb8315ad72118bec6413eb555cbb3a3 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 1 Aug 2025 15:11:43 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Respect=20`max=5Ffailures`=20in=20`?= =?UTF-8?q?ParallelProgressReporter`=20=E2=9C=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is so surprisingly simple that I wonder if I (or more corretly Claude) has missed something. I also made the somewhat related tweak to not do the final report (which repeats all failures etc) in the case of an early exit. Fixes #1162 --- CLAUDE.md | 1 + NEWS.md | 1 + R/reporter-progress.R | 32 +++-- tests/testthat/_snaps/reporter-progress.md | 151 +++++++++++---------- tests/testthat/test-reporter-progress.R | 10 ++ 5 files changed, 113 insertions(+), 82 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bf20bcbaf..eba106e55 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,6 +16,7 @@ General advice: - `devtools::test()` - Run all tests - `devtools::test_file("tests/testthat/test-filename.R")` - Run tests in a specific file +- DO NOT USE `devtools::test_active_file()` - `devtools::load_all()` - Load package for development - `devtools::document()` - Generate documentation - `devtools::check()` - Run R CMD check diff --git a/NEWS.md b/NEWS.md index fdc726e12..c753b28f0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # testthat (development version) +* `ParallelProgressReporter` now respect `max_failures` (#1162). * `JunitReporter()` no longer fails with `"no applicable method for xml_add_child"` for warnings outside of tests (#1913). Additionally, warnings now save their backtraces. * `JunitReporter()` strips ANSI escapes in more placese (#1852, #2032). * `try_again()` is now publicised. The first argument is now the number of retries, not tries (#2050). diff --git a/R/reporter-progress.R b/R/reporter-progress.R index a5405f483..25faf9d10 100644 --- a/R/reporter-progress.R +++ b/R/reporter-progress.R @@ -221,16 +221,20 @@ ProgressReporter <- R6::R6Class( self$report_issues(self$ctxt_issues) if (self$is_full()) { - snapshotter <- get_snapshotter() - if (!is.null(snapshotter)) { - snapshotter$end_file() - } + self$report_full() + } + }, - stop_reporter(c( - "Maximum number of failures exceeded; quitting at end of file.", - i = "Increase this number with (e.g.) {.run testthat::set_max_fails(Inf)}" - )) + report_full = function() { + snapshotter <- get_snapshotter() + if (!is.null(snapshotter)) { + snapshotter$end_file() } + + stop_reporter(c( + "Maximum number of failures exceeded; quitting at end of file.", + i = "Increase this number with (e.g.) {.run testthat::set_max_fails(Inf)}" + )) }, add_result = function(context, test, result) { @@ -258,6 +262,10 @@ ProgressReporter <- R6::R6Class( }, end_reporter = function() { + if (self$is_full()) { + return() + } + self$cat_line() colour_if <- function(n, type) { @@ -465,7 +473,13 @@ ParallelProgressReporter <- R6::R6Class( self$report_issues(fsts$issues) self$files[[self$file_name]] <- NULL - if (length(self$files)) self$update(force = TRUE) + if (length(self$files)) { + self$update(force = TRUE) + } + + if (self$is_full()) { + self$report_full() + } }, end_reporter = function() { diff --git a/tests/testthat/_snaps/reporter-progress.md b/tests/testthat/_snaps/reporter-progress.md index e27a9e2b4..af5ed43d0 100644 --- a/tests/testthat/_snaps/reporter-progress.md +++ b/tests/testthat/_snaps/reporter-progress.md @@ -133,79 +133,6 @@ -------------------------------------------------------------------------------- Maximum number of failures exceeded; quitting at end of file. i Increase this number with (e.g.) `testthat::set_max_fails(Inf)` - - == Results ===================================================================== - -- Failed tests ---------------------------------------------------------------- - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - Failure ('reporters/fail-many.R:3:5'): Example - FALSE (`actual`) is not equal to TRUE (`expected`). - - `actual`: FALSE - `expected`: TRUE - - [ FAIL 11 | WARN 0 | SKIP 0 | PASS 0 ] - == Terminated early ============================================================ - - No one gets it right on their first try # can fully suppress incremental updates @@ -632,3 +559,81 @@ [ FAIL 0 | WARN 0 | SKIP 0 | PASS 6 ] [ FAIL 0 | WARN 0 | SKIP 0 | PASS 7 ] +# ParallelProgressReporter fails after max_fail tests + + v | F W S OK | Context + + - [ FAIL 0 | WARN 0 | SKIP 0 | PASS 0 ] Starting up... + x | 11 0 | reporters/fail-many + -------------------------------------------------------------------------------- + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + + Failure ('reporters/fail-many.R:3:5'): Example + FALSE (`actual`) is not equal to TRUE (`expected`). + + `actual`: FALSE + `expected`: TRUE + -------------------------------------------------------------------------------- + Maximum number of failures exceeded; quitting at end of file. + i Increase this number with (e.g.) `testthat::set_max_fails(Inf)` + + + diff --git a/tests/testthat/test-reporter-progress.R b/tests/testthat/test-reporter-progress.R index bcac0b60f..1c893d081 100644 --- a/tests/testthat/test-reporter-progress.R +++ b/tests/testthat/test-reporter-progress.R @@ -83,3 +83,13 @@ test_that("display of successes only is compact", { test_path("reporters/successes.R") ) }) + +# parallel progress reporter ---------------------------------------------- + +test_that("ParallelProgressReporter fails after max_fail tests", { + withr::local_options(testthat.progress.max_fails = 10) + expect_snapshot_reporter( + ParallelProgressReporter$new(update_interval = 0, min_time = Inf), + test_path(c("reporters/fail-many.R", "reporters/fail.R")) + ) +})