From f34111f806722be7c71b1fa952234d186d5e0eff Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 09:40:56 -0500 Subject: [PATCH 01/12] Bump `the$test_expectations` --- R/test-compiled-code.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 80374ca91..e9fd173f2 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -62,6 +62,7 @@ run_cpp_tests <- function(package) { catch_error <- TRUE reporter <- get_reporter() + the$test_expectations <- the$test_expectations + 1L context_start("Catch") reporter$start_test(context = "Catch", test = "Catch") reporter$add_result( @@ -96,6 +97,7 @@ run_cpp_tests <- function(package) { get_reporter()$start_test(context = context_name, test = test_name) for (i in seq_len(successes)) { + the$test_expectations <- the$test_expectations + 1L exp <- new_expectation("success", "") exp$test <- test_name get_reporter()$add_result( @@ -129,6 +131,7 @@ run_cpp_tests <- function(package) { c(line, line, 1, 1) ) + the$test_expectations <- the$test_expectations + 1L exp <- new_expectation("failure", org_text, srcref = failure_srcref) exp$test <- test_name @@ -141,6 +144,7 @@ run_cpp_tests <- function(package) { exceptions <- xml2::xml_find_all(test, "./Exception") for (exception in exceptions) { + the$test_expectations <- the$test_expectations + 1L exception_text <- xml2::xml_text(exception, trim = TRUE) filename <- xml2::xml_attr(exception, "filename") line <- xml2::xml_attr(exception, "line") From 6d42518a8021829332bbc5fca8d9f8d271f8b2da Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 10:31:40 -0500 Subject: [PATCH 02/12] Move towards less manual bookkeeping --- R/test-compiled-code.R | 173 ++++++++++++++++++++--------------------- R/test-that.R | 4 + 2 files changed, 89 insertions(+), 88 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index e9fd173f2..a1cb77f39 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -60,17 +60,17 @@ run_cpp_tests <- function(package) { }, error = function(e) { catch_error <- TRUE - reporter <- get_reporter() - - the$test_expectations <- the$test_expectations + 1L context_start("Catch") - reporter$start_test(context = "Catch", test = "Catch") - reporter$add_result( - context = "Catch", - test = "Catch", - result = new_expectation("failure", e$message) - ) - reporter$end_test(context = "Catch", test = "Catch") + with_description_push("Catch", { + get_reporter()$start_test(context = "Catch", test = "Catch") + + exp_signal_broken(new_expectation( + "failure", + e$message + )) + + get_reporter()$end_test(context = "Catch", test = "Catch") + }) } ) @@ -91,88 +91,85 @@ run_cpp_tests <- function(package) { for (test in tests) { test_name <- xml2::xml_attr(test, "name") - result <- xml2::xml_find_first(test, "./OverallResults") - successes <- as.integer(xml2::xml_attr(result, "successes")) - - get_reporter()$start_test(context = context_name, test = test_name) - - for (i in seq_len(successes)) { - the$test_expectations <- the$test_expectations + 1L - exp <- new_expectation("success", "") - exp$test <- test_name - get_reporter()$add_result( - context = context_name, - test = test_name, - result = exp - ) - } - - failures <- xml2::xml_find_all(test, "./Expression") - for (failure in failures) { - org <- xml2::xml_find_first(failure, "Original") - org_text <- xml2::xml_text(org, trim = TRUE) - - filename <- xml2::xml_attr(failure, "filename") - type <- xml2::xml_attr(failure, "type") - - type_msg <- switch( - type, - "CATCH_CHECK_FALSE" = "isn't false.", - "CATCH_CHECK_THROWS" = "did not throw an exception.", - "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", - "isn't true." - ) - - org_text <- paste(org_text, type_msg) - - line <- xml2::xml_attr(failure, "line") - failure_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - the$test_expectations <- the$test_expectations + 1L - exp <- new_expectation("failure", org_text, srcref = failure_srcref) - exp$test <- test_name - - get_reporter()$add_result( - context = context_name, - test = test_name, - result = exp - ) - } - - exceptions <- xml2::xml_find_all(test, "./Exception") - for (exception in exceptions) { - the$test_expectations <- the$test_expectations + 1L - exception_text <- xml2::xml_text(exception, trim = TRUE) - filename <- xml2::xml_attr(exception, "filename") - line <- xml2::xml_attr(exception, "line") - - exception_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - exp <- new_expectation( - "error", - exception_text, - srcref = exception_srcref - ) - exp$test <- test_name - - get_reporter()$add_result( - context = context_name, - test = test_name, - result = exp - ) - } - - get_reporter()$end_test(context = context_name, test = test_name) + with_description_push(test_name, { + get_reporter()$start_test(context = context_name, test = test_name) + + result <- xml2::xml_find_first(test, "./OverallResults") + successes <- as.integer(xml2::xml_attr(result, "successes")) + for (i in seq_len(successes)) { + pass() + } + + failures <- xml2::xml_find_all(test, "./Expression") + for (failure in failures) { + org <- xml2::xml_find_first(failure, "Original") + org_text <- xml2::xml_text(org, trim = TRUE) + + filename <- xml2::xml_attr(failure, "filename") + type <- xml2::xml_attr(failure, "type") + + type_msg <- switch( + type, + "CATCH_CHECK_FALSE" = "isn't false.", + "CATCH_CHECK_THROWS" = "did not throw an exception.", + "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", + "isn't true." + ) + + org_text <- paste(org_text, type_msg) + + line <- xml2::xml_attr(failure, "line") + failure_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + # Signal the failure as a condition (not an error) so we can report + # multiple failures without stopping, this use case prevents us from + # being able to use `fail()` outright, since `expectation()` will call + # `stop()` on failures. The expectation handler will catch our + # signaled `"failure"` and properly register it. + exp_signal_broken(new_expectation( + "failure", + org_text, + srcref = failure_srcref + )) + } + + exceptions <- xml2::xml_find_all(test, "./Exception") + for (exception in exceptions) { + exception_text <- xml2::xml_text(exception, trim = TRUE) + filename <- xml2::xml_attr(exception, "filename") + line <- xml2::xml_attr(exception, "line") + + exception_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + exp_signal_broken(new_expectation( + "error", + exception_text, + srcref = exception_srcref + )) + } + + get_reporter()$end_test(context = context_name, test = test_name) + }) } } } +# Like `exp_signal()`, but without `stop()`ing on "broken" expectations, i.e. +# failures/errors as reported by `expectation_broken()`. This allows C++ tests +# to report multiple failures without stopping. +exp_signal_broken <- function(exp) { + withRestarts( + signalCondition(exp), + muffle_expectation = function(e) NULL + ) +} + #' Use Catch for C++ unit testing #' #' Add the necessary infrastructure to enable C++ unit testing diff --git a/R/test-that.R b/R/test-that.R index b33ee8b61..7a22fe8ab 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -233,6 +233,10 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { # Maintain a stack of descriptions +with_description_push <- function(description, expr, frame = caller_env()) { + local_description_push(description = description, frame = frame) + expr +} local_description_push <- function(description, frame = caller_env()) { check_string(description, call = frame) local_description_set(c(the$description, description), frame = frame) From 546436e1c4b57cc3e9c05fe2bc865f9b54d6aabf Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 10:37:43 -0500 Subject: [PATCH 03/12] Fix `with_description_push()` thinko --- R/test-that.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/test-that.R b/R/test-that.R index 7a22fe8ab..4dfc331a2 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -233,8 +233,8 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { # Maintain a stack of descriptions -with_description_push <- function(description, expr, frame = caller_env()) { - local_description_push(description = description, frame = frame) +with_description_push <- function(description, expr) { + local_description_push(description = description) expr } local_description_push <- function(description, frame = caller_env()) { From aac4ad6f323de9640b5d4822fef2cc8ceebb6d74 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 10:40:11 -0500 Subject: [PATCH 04/12] Use `test_code()` to avoid more internals! This also gets the `test_description()` correct so we don't report "code run outside of test_that" --- R/test-compiled-code.R | 150 ++++++++++++++++++++--------------------- 1 file changed, 75 insertions(+), 75 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index a1cb77f39..f8d89fe74 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -48,6 +48,8 @@ run_cpp_tests <- function(package) { run_testthat_tests <- get_routine(package, "run_testthat_tests") + env <- caller_env() + output <- "" tests_passed <- TRUE @@ -61,16 +63,15 @@ run_cpp_tests <- function(package) { error = function(e) { catch_error <- TRUE context_start("Catch") - with_description_push("Catch", { - get_reporter()$start_test(context = "Catch", test = "Catch") - - exp_signal_broken(new_expectation( - "failure", - e$message - )) - - get_reporter()$end_test(context = "Catch", test = "Catch") - }) + with_description_push( + "Catch", + test_code(env = env, { + exp_signal_broken(new_expectation( + "failure", + e$message + )) + }) + ) } ) @@ -91,71 +92,70 @@ run_cpp_tests <- function(package) { for (test in tests) { test_name <- xml2::xml_attr(test, "name") - with_description_push(test_name, { - get_reporter()$start_test(context = context_name, test = test_name) - - result <- xml2::xml_find_first(test, "./OverallResults") - successes <- as.integer(xml2::xml_attr(result, "successes")) - for (i in seq_len(successes)) { - pass() - } - - failures <- xml2::xml_find_all(test, "./Expression") - for (failure in failures) { - org <- xml2::xml_find_first(failure, "Original") - org_text <- xml2::xml_text(org, trim = TRUE) - - filename <- xml2::xml_attr(failure, "filename") - type <- xml2::xml_attr(failure, "type") - - type_msg <- switch( - type, - "CATCH_CHECK_FALSE" = "isn't false.", - "CATCH_CHECK_THROWS" = "did not throw an exception.", - "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", - "isn't true." - ) - - org_text <- paste(org_text, type_msg) - - line <- xml2::xml_attr(failure, "line") - failure_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - # Signal the failure as a condition (not an error) so we can report - # multiple failures without stopping, this use case prevents us from - # being able to use `fail()` outright, since `expectation()` will call - # `stop()` on failures. The expectation handler will catch our - # signaled `"failure"` and properly register it. - exp_signal_broken(new_expectation( - "failure", - org_text, - srcref = failure_srcref - )) - } - - exceptions <- xml2::xml_find_all(test, "./Exception") - for (exception in exceptions) { - exception_text <- xml2::xml_text(exception, trim = TRUE) - filename <- xml2::xml_attr(exception, "filename") - line <- xml2::xml_attr(exception, "line") - - exception_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - exp_signal_broken(new_expectation( - "error", - exception_text, - srcref = exception_srcref - )) - } - - get_reporter()$end_test(context = context_name, test = test_name) - }) + with_description_push( + test_name, + test_code(env = env, { + result <- xml2::xml_find_first(test, "./OverallResults") + successes <- as.integer(xml2::xml_attr(result, "successes")) + for (i in seq_len(successes)) { + pass() + } + + failures <- xml2::xml_find_all(test, "./Expression") + for (failure in failures) { + org <- xml2::xml_find_first(failure, "Original") + org_text <- xml2::xml_text(org, trim = TRUE) + + filename <- xml2::xml_attr(failure, "filename") + type <- xml2::xml_attr(failure, "type") + + type_msg <- switch( + type, + "CATCH_CHECK_FALSE" = "isn't false.", + "CATCH_CHECK_THROWS" = "did not throw an exception.", + "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", + "isn't true." + ) + + org_text <- paste(org_text, type_msg) + + line <- xml2::xml_attr(failure, "line") + failure_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + # Signal the failure as a condition (not an error) so we can report + # multiple failures without stopping, this use case prevents us from + # being able to use `fail()` outright, since `expectation()` will call + # `stop()` on failures. The expectation handler will catch our + # signaled `"failure"` and properly register it. + exp_signal_broken(new_expectation( + "failure", + org_text, + srcref = failure_srcref + )) + } + + exceptions <- xml2::xml_find_all(test, "./Exception") + for (exception in exceptions) { + exception_text <- xml2::xml_text(exception, trim = TRUE) + filename <- xml2::xml_attr(exception, "filename") + line <- xml2::xml_attr(exception, "line") + + exception_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + exp_signal_broken(new_expectation( + "error", + exception_text, + srcref = exception_srcref + )) + } + }) + ) } } } From 99dde7831112d7abf95ba7682cdb49720d9559a2 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 10:45:14 -0500 Subject: [PATCH 05/12] Recognize that `with_description_push()` + `test_code()` is exactly `test_that()`! --- R/test-compiled-code.R | 148 +++++++++++++++++++---------------------- R/test-that.R | 4 -- 2 files changed, 70 insertions(+), 82 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index f8d89fe74..2a22d1b21 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -48,8 +48,6 @@ run_cpp_tests <- function(package) { run_testthat_tests <- get_routine(package, "run_testthat_tests") - env <- caller_env() - output <- "" tests_passed <- TRUE @@ -63,15 +61,12 @@ run_cpp_tests <- function(package) { error = function(e) { catch_error <- TRUE context_start("Catch") - with_description_push( - "Catch", - test_code(env = env, { - exp_signal_broken(new_expectation( - "failure", - e$message - )) - }) - ) + test_that("Catch", { + exp_signal_broken(new_expectation( + "failure", + e$message + )) + }) } ) @@ -85,77 +80,74 @@ run_cpp_tests <- function(package) { for (context in contexts) { context_name <- sub(" [|][^|]+$", "", xml2::xml_attr(context, "name")) - context_start(context_name) tests <- xml2::xml_find_all(context, "./Section") + for (test in tests) { - test_name <- xml2::xml_attr(test, "name") - - with_description_push( - test_name, - test_code(env = env, { - result <- xml2::xml_find_first(test, "./OverallResults") - successes <- as.integer(xml2::xml_attr(result, "successes")) - for (i in seq_len(successes)) { - pass() - } - - failures <- xml2::xml_find_all(test, "./Expression") - for (failure in failures) { - org <- xml2::xml_find_first(failure, "Original") - org_text <- xml2::xml_text(org, trim = TRUE) - - filename <- xml2::xml_attr(failure, "filename") - type <- xml2::xml_attr(failure, "type") - - type_msg <- switch( - type, - "CATCH_CHECK_FALSE" = "isn't false.", - "CATCH_CHECK_THROWS" = "did not throw an exception.", - "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", - "isn't true." - ) - - org_text <- paste(org_text, type_msg) - - line <- xml2::xml_attr(failure, "line") - failure_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - # Signal the failure as a condition (not an error) so we can report - # multiple failures without stopping, this use case prevents us from - # being able to use `fail()` outright, since `expectation()` will call - # `stop()` on failures. The expectation handler will catch our - # signaled `"failure"` and properly register it. - exp_signal_broken(new_expectation( - "failure", - org_text, - srcref = failure_srcref - )) - } - - exceptions <- xml2::xml_find_all(test, "./Exception") - for (exception in exceptions) { - exception_text <- xml2::xml_text(exception, trim = TRUE) - filename <- xml2::xml_attr(exception, "filename") - line <- xml2::xml_attr(exception, "line") - - exception_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - exp_signal_broken(new_expectation( - "error", - exception_text, - srcref = exception_srcref - )) - } - }) - ) + test_description <- xml2::xml_attr(test, "name") + + test_that(test_description, { + result <- xml2::xml_find_first(test, "./OverallResults") + successes <- as.integer(xml2::xml_attr(result, "successes")) + for (i in seq_len(successes)) { + pass() + } + + failures <- xml2::xml_find_all(test, "./Expression") + for (failure in failures) { + org <- xml2::xml_find_first(failure, "Original") + org_text <- xml2::xml_text(org, trim = TRUE) + + filename <- xml2::xml_attr(failure, "filename") + type <- xml2::xml_attr(failure, "type") + + type_msg <- switch( + type, + "CATCH_CHECK_FALSE" = "isn't false.", + "CATCH_CHECK_THROWS" = "did not throw an exception.", + "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", + "isn't true." + ) + + org_text <- paste(org_text, type_msg) + + line <- xml2::xml_attr(failure, "line") + failure_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + # Signal the failure as a condition (not an error) so we can report + # multiple failures without stopping, this use case prevents us from + # being able to use `fail()` outright, since `expectation()` will call + # `stop()` on failures. The expectation handler will catch our + # signaled `"failure"` and properly register it. + exp_signal_broken(new_expectation( + "failure", + org_text, + srcref = failure_srcref + )) + } + + exceptions <- xml2::xml_find_all(test, "./Exception") + for (exception in exceptions) { + exception_text <- xml2::xml_text(exception, trim = TRUE) + filename <- xml2::xml_attr(exception, "filename") + line <- xml2::xml_attr(exception, "line") + + exception_srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + exp_signal_broken(new_expectation( + "error", + exception_text, + srcref = exception_srcref + )) + } + }) } } } diff --git a/R/test-that.R b/R/test-that.R index 4dfc331a2..b33ee8b61 100644 --- a/R/test-that.R +++ b/R/test-that.R @@ -233,10 +233,6 @@ test_code <- function(code, env, reporter = NULL, skip_on_empty = TRUE) { # Maintain a stack of descriptions -with_description_push <- function(description, expr) { - local_description_push(description = description) - expr -} local_description_push <- function(description, frame = caller_env()) { check_string(description, call = frame) local_description_set(c(the$description, description), frame = frame) From a9e931972677c6c916257aedb7af80262732d6ec Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 11:05:16 -0500 Subject: [PATCH 06/12] Recognize that we can now use `fail()` Since we are now inside a `test_that()`, which has its own local `test_code()`, where each of those has its own `tryCatch()` --- R/test-compiled-code.R | 47 ++++++++++++------------------------------ 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 2a22d1b21..eac3c859d 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -51,7 +51,8 @@ run_cpp_tests <- function(package) { output <- "" tests_passed <- TRUE - catch_error <- FALSE + catch_error <- NULL + tryCatch( { output <- capture_output_lines( @@ -59,18 +60,15 @@ run_cpp_tests <- function(package) { ) }, error = function(e) { - catch_error <- TRUE - context_start("Catch") - test_that("Catch", { - exp_signal_broken(new_expectation( - "failure", - e$message - )) - }) + catch_error <- e } ) - if (catch_error) { + if (!is.null(catch_error)) { + context_start("Catch") + test_that("Catch", { + fail(catch_error$message) + }) return() } @@ -118,16 +116,7 @@ run_cpp_tests <- function(package) { c(line, line, 1, 1) ) - # Signal the failure as a condition (not an error) so we can report - # multiple failures without stopping, this use case prevents us from - # being able to use `fail()` outright, since `expectation()` will call - # `stop()` on failures. The expectation handler will catch our - # signaled `"failure"` and properly register it. - exp_signal_broken(new_expectation( - "failure", - org_text, - srcref = failure_srcref - )) + fail(org_text, srcref = failure_srcref) } exceptions <- xml2::xml_find_all(test, "./Exception") @@ -141,27 +130,17 @@ run_cpp_tests <- function(package) { c(line, line, 1, 1) ) - exp_signal_broken(new_expectation( - "error", - exception_text, + expectation( + type = "error", + message = exception_text, srcref = exception_srcref - )) + ) } }) } } } -# Like `exp_signal()`, but without `stop()`ing on "broken" expectations, i.e. -# failures/errors as reported by `expectation_broken()`. This allows C++ tests -# to report multiple failures without stopping. -exp_signal_broken <- function(exp) { - withRestarts( - signalCondition(exp), - muffle_expectation = function(e) NULL - ) -} - #' Use Catch for C++ unit testing #' #' Add the necessary infrastructure to enable C++ unit testing From 09482abf5ba127c1e1486df159182053bbf8896b Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 11:05:37 -0500 Subject: [PATCH 07/12] You really do need `<<-` here! --- R/test-compiled-code.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index eac3c859d..dff00d69d 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -60,7 +60,7 @@ run_cpp_tests <- function(package) { ) }, error = function(e) { - catch_error <- e + catch_error <<- e } ) From 2c12010979174db4ad86262eea73a6a0c9bb59d6 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 11:52:33 -0500 Subject: [PATCH 08/12] Use `conditionMessage()` just in case --- R/test-compiled-code.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index dff00d69d..7016aaf94 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -67,7 +67,7 @@ run_cpp_tests <- function(package) { if (!is.null(catch_error)) { context_start("Catch") test_that("Catch", { - fail(catch_error$message) + fail(conditionMessage(catch_error)) }) return() } From 18d248cdf871f9523b818ddd8fe89ba4d0571d84 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 12:03:48 -0500 Subject: [PATCH 09/12] Justify usage of `expectation()` --- R/test-compiled-code.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 7016aaf94..780023688 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -130,6 +130,8 @@ run_cpp_tests <- function(package) { c(line, line, 1, 1) ) + # There is no `fail()` equivalent for an error. + # We could use `stop()`, but we want to pass through a `srcref`. expectation( type = "error", message = exception_text, From 3b17e839c151fe43e44a4b62e6505681792d5dd4 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 12:49:30 -0500 Subject: [PATCH 10/12] NEWS bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 87080ac5b..d7c832029 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # testthat (development version) +* `run_cpp_tests()` no longer accidentally reports that a test has been skipped (#2315). + # testthat 3.3.2 * testthat now emits OpenTelemetry traces for tests when tracing is enabled. Requires the otel and otelsdk packages (#2282). From b8c19425dd1531e5eb5156b3fb3469c60987a0a7 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 15:23:39 -0500 Subject: [PATCH 11/12] Refactor into a series of parsers --- R/test-compiled-code.R | 143 +++++++++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 54 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 780023688..395a834ff 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -72,70 +72,27 @@ run_cpp_tests <- function(package) { return() } - report <- xml2::read_xml(paste(output, collapse = "\n")) - - contexts <- xml2::xml_find_all(report, "//TestCase") + output <- paste(output, collapse = "\n") + contexts <- parse_catch_contexts(output) for (context in contexts) { - context_name <- sub(" [|][^|]+$", "", xml2::xml_attr(context, "name")) - context_start(context_name) - - tests <- xml2::xml_find_all(context, "./Section") - - for (test in tests) { - test_description <- xml2::xml_attr(test, "name") + context_start(context$name) - test_that(test_description, { - result <- xml2::xml_find_first(test, "./OverallResults") - successes <- as.integer(xml2::xml_attr(result, "successes")) - for (i in seq_len(successes)) { + for (test in context$tests) { + test_that(test$name, { + for (i in seq_len(test$n_successes)) { pass() } - - failures <- xml2::xml_find_all(test, "./Expression") - for (failure in failures) { - org <- xml2::xml_find_first(failure, "Original") - org_text <- xml2::xml_text(org, trim = TRUE) - - filename <- xml2::xml_attr(failure, "filename") - type <- xml2::xml_attr(failure, "type") - - type_msg <- switch( - type, - "CATCH_CHECK_FALSE" = "isn't false.", - "CATCH_CHECK_THROWS" = "did not throw an exception.", - "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", - "isn't true." - ) - - org_text <- paste(org_text, type_msg) - - line <- xml2::xml_attr(failure, "line") - failure_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - - fail(org_text, srcref = failure_srcref) + for (failure in test$failures) { + fail(message = failure$message, srcref = failure$srcref) } - - exceptions <- xml2::xml_find_all(test, "./Exception") - for (exception in exceptions) { - exception_text <- xml2::xml_text(exception, trim = TRUE) - filename <- xml2::xml_attr(exception, "filename") - line <- xml2::xml_attr(exception, "line") - - exception_srcref <- srcref( - srcfile(file.path("src", filename)), - c(line, line, 1, 1) - ) - + for (exception in test$exceptions) { # There is no `fail()` equivalent for an error. # We could use `stop()`, but we want to pass through a `srcref`. expectation( type = "error", - message = exception_text, - srcref = exception_srcref + message = exception$message, + srcref = exception$srcref ) } }) @@ -143,6 +100,84 @@ run_cpp_tests <- function(package) { } } +parse_catch_contexts <- function(text) { + xml <- xml2::read_xml(text) + + contexts <- xml2::xml_find_all(xml, "//TestCase") + contexts <- map(contexts, parse_catch_context) + + contexts +} + +parse_catch_context <- function(context) { + name <- sub(" [|][^|]+$", "", xml2::xml_attr(context, "name")) + tests <- xml2::xml_find_all(context, "./Section") + tests <- map(tests, parse_catch_test) + list(name = name, tests = tests) +} + +parse_catch_test <- function(test) { + name <- xml2::xml_attr(test, "name") + + overall_results <- xml2::xml_find_first(test, "./OverallResults") + n_successes <- as.integer(xml2::xml_attr(overall_results, "successes")) + + failures <- xml2::xml_find_all(test, "./Expression") + failures <- map(failures, parse_catch_failure) + + exceptions <- xml2::xml_find_all(test, "./Exception") + exceptions <- map(exceptions, parse_catch_exception) + + list( + name = name, + n_successes = n_successes, + failures = failures, + exceptions = exceptions + ) +} + +parse_catch_failure <- function(failure) { + type <- switch( + xml2::xml_attr(failure, "type"), + "CATCH_CHECK_FALSE" = "isn't false.", + "CATCH_CHECK_THROWS" = "did not throw an exception.", + "CATCH_CHECK_THROWS_AS" = "threw an exception with unexpected type.", + "isn't true." + ) + + message <- xml2::xml_find_first(failure, "Original") + message <- xml2::xml_text(message, trim = TRUE) + message <- paste(message, type) + + filename <- xml2::xml_attr(failure, "filename") + line <- xml2::xml_attr(failure, "line") + srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + list( + message = message, + srcref = srcref + ) +} + +parse_catch_exception <- function(exception) { + message <- xml2::xml_text(exception, trim = TRUE) + + filename <- xml2::xml_attr(exception, "filename") + line <- xml2::xml_attr(exception, "line") + srcref <- srcref( + srcfile(file.path("src", filename)), + c(line, line, 1, 1) + ) + + list( + message = message, + srcref = srcref + ) +} + #' Use Catch for C++ unit testing #' #' Add the necessary infrastructure to enable C++ unit testing From a9d4463702d0e230e49614391b194e04ecf24375 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Wed, 21 Jan 2026 15:25:54 -0500 Subject: [PATCH 12/12] Remove unused variable --- R/test-compiled-code.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/test-compiled-code.R b/R/test-compiled-code.R index 395a834ff..ba647ca5b 100644 --- a/R/test-compiled-code.R +++ b/R/test-compiled-code.R @@ -49,14 +49,12 @@ run_cpp_tests <- function(package) { run_testthat_tests <- get_routine(package, "run_testthat_tests") output <- "" - tests_passed <- TRUE - catch_error <- NULL tryCatch( { output <- capture_output_lines( - tests_passed <- .Call(run_testthat_tests, TRUE) + .Call(run_testthat_tests, TRUE) ) }, error = function(e) {