Handle syntax errors in parallel runs (#2172)

Author: gaborcsardi

NEWS.md

@@ -1,12 +1,13 @@
 # testthat (development version)
 
+* Parallel testthat now does not ignore test files with syntax errors (#1360).
 * `expect_lt()`, `expect_gt()`, and friends have a refined display that is more likely to display the correct number of digits and shows you the actual values compared.
 * `describe()`, `it()`, and `test_that()` now have a shared stack of descriptions so that if you nest any inside of each other, any resulting failures will show you the full path.
 * `describe()` now correctly scopes `skip()` (#2007).
 * `ParallelProgressReporter` now respect `max_failures` (#1162).
 * The last snapshot is no longer lost if the snapshot file is missing the final newline (#2092). It's easy to accidentally remove this because there are two trailing new lines in snapshot files and many editors will automatically remove if you touch the file.
 * New `expect_r6_class()` (#2030).
-* `expect_*()` functions consistently and rigorously check their inputs (#1754). 
+* `expect_*()` functions consistently and rigorously check their inputs (#1754).
 * `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).

R/parallel-taskq.R

@@ -78,6 +78,9 @@ task_q <- R6::R6Class(
             private$tasks$state[[i]] <- "ready"
             msg <- NULL
           } else if (msg$code == PROCESS_DONE) {
+            if (!is.null(msg$error)) {
+              private$handle_error(msg, i)
+            }
             private$tasks$state[[i]] <- "ready"
           } else if (msg$code %in% PROCESS_FAILURES) {
             private$handle_error(msg, i)

tests/testthat/test-parallel-errors.R

@@ -0,0 +1,18 @@
+test_that("error in parallel setup code", {
+  skip_on_covr()
+  withr::local_envvar(TESTTHAT_PARALLEL = "TRUE")
+  err <- tryCatch(
+    capture.output(suppressMessages(testthat::test_local(
+      test_path("test-parallel", "syntax-error"),
+      reporter = "summary"
+    ))),
+    error = function(e) e
+  )
+  expect_s3_class(err, "testthat_process_error")
+  # contains test file's name
+  expect_match(conditionMessage(err), "test-error-1.R")
+  # contains original error message
+  expect_match(conditionMessage(err), "unexpected symbol")
+  # contains the text of the syntax error
+  expect_match(conditionMessage(err), "but this")
+})

tests/testthat/test-parallel/syntax-error/DESCRIPTION

@@ -0,0 +1,20 @@
+Package: setup
+Title: What the Package Does (One Line, Title Case)
+Version: 0.0.0.9000
+Authors@R: 
+    person(given = "First",
+           family = "Last",
+           role = c("aut", "cre"),
+           email = "[email protected]",
+           comment = c(ORCID = "YOUR-ORCID-ID"))
+Description: What the package does (one paragraph).
+License: `use_mit_license()`, `use_gpl3_license()` or friends to
+    pick a license
+Encoding: UTF-8
+LazyData: true
+Roxygen: list(markdown = TRUE)
+RoxygenNote: 7.1.1
+Suggests: 
+    testthat
+Config/testthat/parallel: true
+Config/testthat/edition: 3

tests/testthat/test-parallel/syntax-error/NAMESPACE

@@ -0,0 +1,2 @@
+# Generated by roxygen2: do not edit by hand
+

tests/testthat/test-parallel/syntax-error/tests/testthat.R

@@ -0,0 +1,4 @@
+library(testthat)
+library(ok)
+
+test_check("ok")

tests/testthat/test-parallel/syntax-error/tests/testthat/test-error-1.R

@@ -0,0 +1,5 @@
+test_that("this is good", {
+  expect_equal(2 * 2, 4)
+})
+
+but this is a syntax error!

tests/testthat/test-parallel/syntax-error/tests/testthat/test-error-2.R

@@ -0,0 +1,3 @@
+test_that("this fails", {
+  expect_equal(Sys.getpid(), 0L)
+})