Two minor improvements (#590)

* explicit perl and fixed arguments to expect_match() to allow reporting an unescaped label with `fixed = TRUE` * improve behavior of summary reporter in case of overflow * update infinite recursion test for R 3.4.0
r-lib · Jun 6, 2017 · c7e8330 · c7e8330
1 parent 092bded
commit c7e8330
Show file tree

Hide file tree

Showing 18 changed files with 126 additions and 58 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,13 @@
 # testthat 1.0.2.9000
 
+* Improved behavior of the `SummaryReporter` when the maximum number of errors
+  reported is reached.
+
+* `expect_match()` now accepts explicit `perl` and `fixed` arguments, and adapts
+  the failure message to the value of `fixed`. This also affects other expectations
+  that forward to `expect_match()`, like `expect_output()`, `expect_message()`,
+  `expect_warning()`, and `expect_error()`.
+
 * New option `testthat.default_check_reporter`, defaults to `"check"`. 
   Continuous Integration system can set this option before evaluating
   package test sources in order to direct test result details to known

diff --git a/R/expect-output.R b/R/expect-output.R
@@ -10,7 +10,7 @@
 #' that `options(warn)` has no effect.
 #'
 #' @inheritParams expect_that
-#' @inheritParams expect_match
+#' @inheritDotParams expect_match -object -regexp -info -label
 #' @param regexp regular expression to test against.
 #'
 #'   If `NULL`, the default, asserts that there should be an output,

diff --git a/R/expectations-matches.R b/R/expectations-matches.R
@@ -1,11 +1,11 @@
-#' Expectation: does string/output/message/warning/error match a regular expression?
+#' Expectation: does string match a regular expression?
 #'
 #' @inheritParams expect_that
+#' @inheritParams base::grepl
 #' @param regexp Regular expression to test against.
 #' @param all Should all elements of actual value match `regexp` (TRUE),
 #'    or does only one need to match (FALSE)
-#' @param ... Additional arguments passed on to [grepl()], e.g.
-#'   `ignore.case` or `fixed`.
+#' @inheritDotParams base::grepl -pattern -x -perl -fixed
 #' @family expectations
 #' @export
 #' @examples
@@ -18,8 +18,12 @@
 #' # Zero-length inputs always fail
 #' expect_match(character(), ".")
 #' }
-expect_match <- function(object, regexp, ..., all = TRUE,
+expect_match <- function(object, regexp, perl = FALSE, fixed = FALSE, ..., all = TRUE,
                          info = NULL, label = NULL) {
+
+  if (fixed) escape <- identity
+  else escape <- escape_regex
+
   stopifnot(is.character(regexp), length(regexp) == 1)
   label <- make_label(object, label)
 
@@ -28,19 +32,19 @@ expect_match <- function(object, regexp, ..., all = TRUE,
     fail(sprintf("%s is empty.", label))
   }
 
-  matches <- grepl(regexp, object, ...)
+  matches <- grepl(regexp, object, perl = perl, fixed = fixed, ...)
 
   if (length(object) == 1) {
-    values <- paste0("Actual value: \"", escape_regex(encodeString(object)), "\"")
+    values <- paste0("Actual value: \"", escape(encodeString(object)), "\"")
   } else {
     values <- paste0("Actual values:\n",
-      paste0("* ", escape_regex(encodeString(object)), collapse = "\n"))
+      paste0("* ", escape(encodeString(object)), collapse = "\n"))
   }
   expect(
     if (all) all(matches) else any(matches),
     sprintf(
       "%s does not match %s.\n%s",
-      escape_regex(label),
+      escape(label),
       encodeString(regexp, quote = '"'),
       values
     ),

diff --git a/R/reporter-summary.R b/R/reporter-summary.R
@@ -36,15 +36,12 @@ SummaryReporter <- R6::R6Class("SummaryReporter", inherit = Reporter,
       self$omit_dots <- omit_dots
     },
 
-    start_context = function(context) {
-      self$cat_tight(context, ": ")
+    is_full = function() {
+      self$failures$size() >= self$max_reports
     },
 
-    end_test = function(context, test) {
-      if (self$failures$size() >= self$max_reports) {
-        self$cat_line()
-        stop("Reached maximum number of reports.", call. = FALSE)
-      }
+    start_context = function(context) {
+      self$cat_tight(context, ": ")
     },
 
     end_context = function(context) {
@@ -77,6 +74,14 @@ SummaryReporter <- R6::R6Class("SummaryReporter", inherit = Reporter,
       private$cat_reports("Warnings", warnings, Inf, skip_summary)
       private$cat_reports("Failed", failures, self$max_reports, failure_summary)
 
+      if (self$failures$size() >= self$max_reports) {
+        self$cat_line(
+          "Maximum number of ", self$max_reports, " failures reached, ",
+          "some test results may be missing."
+        )
+        self$cat_line()
+      }
+
       self$rule("DONE", pad = "=")
       if (self$show_praise) {
         if (length(failures) == 0 && runif(1) < 0.1) {

diff --git a/R/reporter.R b/R/reporter.R
@@ -17,6 +17,7 @@ Reporter <- R6::R6Class("Reporter",
     end_test =       function(context, test) {},
     end_context =    function(context) {},
     end_reporter =   function() {},
+    is_full =        function() FALSE,
 
     out = NULL,
 

diff --git a/R/test-files.R b/R/test-files.R
@@ -112,6 +112,7 @@ test_file <- function(path, reporter = default_reporter(), env = test_env(),
   library(testthat)
 
   reporter <- find_reporter(reporter)
+  if (reporter$is_full()) return()
 
   if (load_helpers) {
     source_test_helpers(dirname(path), env = env)

diff --git a/man/expect_match.Rd b/man/expect_match.Rd
diff --git a/man/output-expectations.Rd b/man/output-expectations.Rd
diff --git a/tests/testthat/reporters/check.txt b/tests/testthat/reporters/check.txt
@@ -40,16 +40,16 @@ evaluation nested too deeply: infinite recursion / options(expressions=)?
 9: f() at reporters/tests.R:42
 10: f() at reporters/tests.R:42
 ...
-164: f() at reporters/tests.R:42
-165: f() at reporters/tests.R:42
-166: f() at reporters/tests.R:42
-167: f() at reporters/tests.R:42
-168: f() at reporters/tests.R:42
-169: f() at reporters/tests.R:42
 170: f() at reporters/tests.R:42
 171: f() at reporters/tests.R:42
 172: f() at reporters/tests.R:42
 173: f() at reporters/tests.R:42
+174: f() at reporters/tests.R:42
+175: f() at reporters/tests.R:42
+176: f() at reporters/tests.R:42
+177: f() at reporters/tests.R:42
+178: f() at reporters/tests.R:42
+179: f() at reporters/tests.R:42
 
 testthat results ================================================================
 OK: 7 SKIPPED: 3 FAILED: 7

diff --git a/tests/testthat/reporters/debug.txt b/tests/testthat/reporters/debug.txt
@@ -187,6 +187,12 @@
 171: tests.R#42: f()
 172: tests.R#42: f()
 173: tests.R#42: f()
+174: tests.R#42: f()
+175: tests.R#42: f()
+176: tests.R#42: f()
+177: tests.R#42: f()
+178: tests.R#42: f()
+179: tests.R#42: f()
 
 1: tests.R#49: skip("skip")
 

diff --git a/tests/testthat/reporters/junit.txt b/tests/testthat/reporters/junit.txt
@@ -49,16 +49,16 @@
 9: f() at reporters/tests.R:42
 10: f() at reporters/tests.R:42
 ...
-164: f() at reporters/tests.R:42
-165: f() at reporters/tests.R:42
-166: f() at reporters/tests.R:42
-167: f() at reporters/tests.R:42
-168: f() at reporters/tests.R:42
-169: f() at reporters/tests.R:42
 170: f() at reporters/tests.R:42
 171: f() at reporters/tests.R:42
 172: f() at reporters/tests.R:42
-173: f() at reporters/tests.R:42</error>
+173: f() at reporters/tests.R:42
+174: f() at reporters/tests.R:42
+175: f() at reporters/tests.R:42
+176: f() at reporters/tests.R:42
+177: f() at reporters/tests.R:42
+178: f() at reporters/tests.R:42
+179: f() at reporters/tests.R:42</error>
     </testcase>
   </testsuite>
   <testsuite name="Skips" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="3" skipped="3" failures="0" errors="0" time="0">

diff --git a/tests/testthat/reporters/summary-2.txt b/tests/testthat/reporters/summary-2.txt
@@ -1,4 +1,23 @@
-Expectations: .12
+Expectations: .1234.
+Errors: 56
+Recursion: 7
+Skips: SSS
+Warnings: WWW
+Output: ..
+
+Skipped ------------------------------------------------------------------------
+1. Skip:1 (@tests.R#49) - skip
+
+2. Skip:2 (@tests.R#54) - skip
+
+3. Skip:3 (@tests.R#57) - Empty test
+
+Warnings -----------------------------------------------------------------------
+1. Warning:1 (@tests.R#63) - abc
+
+2. Warning:2 (@tests.R#67) - def
+
+3. Warning:2 (@tests.R#68) - ghi
 
 Failed -------------------------------------------------------------------------
 1. Failure: Failure:1 (@tests.R#8) ---------------------------------------------
@@ -7,6 +26,9 @@ Failure has been forced
 
 2. Failure: Failure:2a (@tests.R#12) -------------------------------------------
 Failure has been forced
+  ... and 5 more
+
 
+Maximum number of 2 failures reached, some test results may be missing.
 
 DONE ===========================================================================
diff --git a/tests/testthat/reporters/summary-no-dots.txt b/tests/testthat/reporters/summary-no-dots.txt
@@ -62,15 +62,15 @@ evaluation nested too deeply: infinite recursion / options(expressions=)?
 9: f() at reporters/tests.R:42
 10: f() at reporters/tests.R:42
 ...
-164: f() at reporters/tests.R:42
-165: f() at reporters/tests.R:42
-166: f() at reporters/tests.R:42
-167: f() at reporters/tests.R:42
-168: f() at reporters/tests.R:42
-169: f() at reporters/tests.R:42
 170: f() at reporters/tests.R:42
 171: f() at reporters/tests.R:42
 172: f() at reporters/tests.R:42
 173: f() at reporters/tests.R:42
+174: f() at reporters/tests.R:42
+175: f() at reporters/tests.R:42
+176: f() at reporters/tests.R:42
+177: f() at reporters/tests.R:42
+178: f() at reporters/tests.R:42
+179: f() at reporters/tests.R:42
 
 DONE ===========================================================================
diff --git a/tests/testthat/reporters/summary.txt b/tests/testthat/reporters/summary.txt
@@ -62,15 +62,15 @@ evaluation nested too deeply: infinite recursion / options(expressions=)?
 9: f() at reporters/tests.R:42
 10: f() at reporters/tests.R:42
 ...
-164: f() at reporters/tests.R:42
-165: f() at reporters/tests.R:42
-166: f() at reporters/tests.R:42
-167: f() at reporters/tests.R:42
-168: f() at reporters/tests.R:42
-169: f() at reporters/tests.R:42
 170: f() at reporters/tests.R:42
 171: f() at reporters/tests.R:42
 172: f() at reporters/tests.R:42
 173: f() at reporters/tests.R:42
+174: f() at reporters/tests.R:42
+175: f() at reporters/tests.R:42
+176: f() at reporters/tests.R:42
+177: f() at reporters/tests.R:42
+178: f() at reporters/tests.R:42
+179: f() at reporters/tests.R:42
 
 DONE ===========================================================================
diff --git a/tests/testthat/reporters/tap.txt b/tests/testthat/reporters/tap.txt
@@ -40,16 +40,16 @@ not ok 9 Recursion:1
   9: f() at reporters/tests.R:42
   10: f() at reporters/tests.R:42
   ...
-  164: f() at reporters/tests.R:42
-  165: f() at reporters/tests.R:42
-  166: f() at reporters/tests.R:42
-  167: f() at reporters/tests.R:42
-  168: f() at reporters/tests.R:42
-  169: f() at reporters/tests.R:42
   170: f() at reporters/tests.R:42
   171: f() at reporters/tests.R:42
   172: f() at reporters/tests.R:42
   173: f() at reporters/tests.R:42
+  174: f() at reporters/tests.R:42
+  175: f() at reporters/tests.R:42
+  176: f() at reporters/tests.R:42
+  177: f() at reporters/tests.R:42
+  178: f() at reporters/tests.R:42
+  179: f() at reporters/tests.R:42
 # Context Skips
 ok 10 # SKIP skip
 ok 11 # SKIP skip

diff --git a/tests/testthat/reporters/teamcity.txt b/tests/testthat/reporters/teamcity.txt
@@ -52,7 +52,7 @@
 ##teamcity[testSuiteStarted name='Recursion']
 ##teamcity[testSuiteStarted name='Recursion:1']
 ##teamcity[testStarted name='expectation 1']
-##teamcity[testFailed name='expectation 1' message='evaluation nested too deeply: infinite recursion / options(expressions=)?' details='1: f() at reporters/tests.R:43|n2: f() at reporters/tests.R:42|n3: f() at reporters/tests.R:42|n4: f() at reporters/tests.R:42|n5: f() at reporters/tests.R:42|n6: f() at reporters/tests.R:42|n7: f() at reporters/tests.R:42|n8: f() at reporters/tests.R:42|n9: f() at reporters/tests.R:42|n10: f() at reporters/tests.R:42|n...|n164: f() at reporters/tests.R:42|n165: f() at reporters/tests.R:42|n166: f() at reporters/tests.R:42|n167: f() at reporters/tests.R:42|n168: f() at reporters/tests.R:42|n169: f() at reporters/tests.R:42|n170: f() at reporters/tests.R:42|n171: f() at reporters/tests.R:42|n172: f() at reporters/tests.R:42|n173: f() at reporters/tests.R:42']
+##teamcity[testFailed name='expectation 1' message='evaluation nested too deeply: infinite recursion / options(expressions=)?' details='1: f() at reporters/tests.R:43|n2: f() at reporters/tests.R:42|n3: f() at reporters/tests.R:42|n4: f() at reporters/tests.R:42|n5: f() at reporters/tests.R:42|n6: f() at reporters/tests.R:42|n7: f() at reporters/tests.R:42|n8: f() at reporters/tests.R:42|n9: f() at reporters/tests.R:42|n10: f() at reporters/tests.R:42|n...|n170: f() at reporters/tests.R:42|n171: f() at reporters/tests.R:42|n172: f() at reporters/tests.R:42|n173: f() at reporters/tests.R:42|n174: f() at reporters/tests.R:42|n175: f() at reporters/tests.R:42|n176: f() at reporters/tests.R:42|n177: f() at reporters/tests.R:42|n178: f() at reporters/tests.R:42|n179: f() at reporters/tests.R:42']
 ##teamcity[testFinished name='expectation 1']
 ##teamcity[testSuiteFinished name='Recursion:1']
 

diff --git a/tests/testthat/test-expect-match.R b/tests/testthat/test-expect-match.R
@@ -10,3 +10,7 @@ test_that("special regex characters are escaped in output", {
   expect_equal(error, "\"f\\(\\) test\" does not match \"f() test\".\nActual value: \"f\\(\\) test\"\n")
 })
 
+test_that("correct reporting of expected label", {
+  expect_failure(expect_match("[a]", "[b]"), escape_regex("[a]"), fixed = TRUE)
+  expect_failure(expect_match("[a]", "[b]", fixed = TRUE), "[a]", fixed = TRUE)
+})
diff --git a/tests/testthat/test-reporter.R b/tests/testthat/test-reporter.R
@@ -67,8 +67,7 @@ test_that("reporters produce consistent output", {
   )
   save_report("check", error_regexp = NULL)
   save_report("summary", SummaryReporter$new(show_praise = FALSE, omit_dots = FALSE))
-  save_report("summary-2", SummaryReporter$new(show_praise = FALSE, max_reports = 2),
-              error_regexp = "Reached maximum number of reports")
+  save_report("summary-2", SummaryReporter$new(show_praise = FALSE, max_reports = 2))
   save_report("summary-no-dots", SummaryReporter$new(show_praise = FALSE, omit_dots = TRUE))
   save_report("location")
   save_report("minimal")