Adds name attribute to <testcase> elements in junit output (#575) (#576)

* Adds name attribute to <testcase> elements in junit output * Updated junit test data * Removed encoding attribute from xml tag in junit results Perhaps a different version of libxml is being used than to generate the original output. * Fixed indenting * Fix JUnit output - "message" attribute mapped directly to expectation message - failure/error body set to format(expectation) This produces the same output as the summary reporter for errors/failures. * Fixed expected junit output paths * Fixed (again) junit output Matched the stack depth numbers to those output by the summary reporter. * Allow configuration of test_check() using options() This adds two options: * testthat.default_check_reporter: allows configuration of a different reporter to use for test_check(). For example, 'junit' * testthat.junit.output_file: specifies a file to which the junit xml should be written. These are needed so that Ci systems can run test scripts in R packages without modification, but first set these options() so that test output will be written to a known location where it can be read and parsed. * Updated NEWS.md with changes to JUnitReporter and new options
r-lib · Feb 18, 2017 · cb7cae0 · cb7cae0
1 parent 84493d1
commit cb7cae0
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 38 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,19 @@
 # testthat 1.0.2.9000
 
+* 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
+  location.
+
+* New option `testthat.junit.output_file`. If set, the JUnitReporter
+  will write the test results to the provided path rather than 
+  standard output.
+
+* Fixed JUnitReporter output format (#575). The testcase element now
+  includes both the `classname` attribute, which contains the testhat 
+  context, and the `name` attriute, which contains the testthat 
+  test name.
+
 * The default summary reporter aborts testing as soon as the limit given by the
   option `testthat.summary.max_reports` (default 15) is reached
   (#520).

diff --git a/R/reporter-junit.R b/R/reporter-junit.R
@@ -88,42 +88,46 @@ JunitReporter <- R6::R6Class("JunitReporter", inherit = Reporter,
       # XML node for test case
       name <- test %||% "(unnamed)"
       testcase <- xml2::xml_add_child(self$suite, "testcase",
-       time = toString(time),
-       classname = paste0(classnameOK(context), '.', classnameOK(name))
+        time = toString(time),
+        classname = classnameOK(context),
+        name = classnameOK(name)
       )
 
-      # message - if failure or error
-      message <- if (is.null(result$call)) "(unexpected)" else format(result$call)[1]
-
-      if (!is.null(result$srcref)) {
-        location <- paste0('@', attr(result$srcref, 'srcfile')$filename, '#', result$srcref[1])
-        message  <- paste(as.character(result), location)
+      first_line <- function(x) {
+        strsplit(x, split = "\n")[[1]][1]
       }
 
       # add an extra XML child node if not a success
       if (expectation_error(result)) {
         # "type" in Java is the exception class
-        xml2::xml_add_child(testcase, 'error', type = 'error', message = message)
+        error <- xml2::xml_add_child(testcase, 'error', type = 'error', message = first_line(result$message))
+        xml2::xml_text(error) <- format(result)
         self$errors <- self$errors + 1
+
       } else if (expectation_failure(result)) {
         # "type" in Java is the type of assertion that failed
-        xml2::xml_add_child(testcase, 'failure', type = 'failure', message = message)
+        failure <- xml2::xml_add_child(testcase, 'failure', type = 'failure', message = first_line(result$message))
+        xml2::xml_text(failure) <- format(result)
         self$failures <- self$failures + 1
+
       } else if (expectation_skip(result)) {
         xml2::xml_add_child(testcase, "skipped")
         self$skipped <- self$skipped + 1
       }
     },
 
     end_reporter = function() {
-      if (inherits(self$out, "connection")) {
+      output_file <- getOption("testthat.junit.output_file")
+
+      if (!is.null(output_file)) {
+        xml2::write_xml(self$doc, output_file, format = TRUE)
+      } else if (inherits(self$out, "connection")) {
         file <- tempfile()
         xml2::write_xml(self$doc, file, format = TRUE)
         writeLines(readLines(file), self$out)
       } else {
         stop('unsupported output type: ', toString(self$out))
       }
-      #cat(toString(self$doc), file = self$file)
     } # end_reporter
   ), #public
 

diff --git a/R/test-package.R b/R/test-package.R
@@ -77,7 +77,7 @@ run_tests <- function(package, test_path, filter, reporter, ...)
 #' @inheritParams test_package
 #' @export
 #' @rdname test_package
-test_check <- function(package, filter = NULL, reporter = "check", ...) {
+test_check <- function(package, filter = NULL, reporter = getOption("testthat.default_check_reporter", "check"), ...) {
   library(testthat)
   require(package, character.only = TRUE)
 

diff --git a/tests/testthat/reporters/junit.txt b/tests/testthat/reporters/junit.txt
@@ -1,52 +1,84 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
   <testsuite name="Expectations" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="6" skipped="0" failures="4" errors="0" time="0">
-    <testcase time="0" classname="Expectations.Success"/>
-    <testcase time="0" classname="Expectations.Failure:1">
-      <failure type="failure" message="expectation_failure in structure(list(fail()), start_frame = 57): Failure has been forced&#10;&#10; @reporters/tests.R#8"/>
+    <testcase time="0" classname="Expectations" name="Success"/>
+    <testcase time="0" classname="Expectations" name="Failure:1">
+      <failure type="failure" message="Failure has been forced">Failure has been forced
+</failure>
     </testcase>
-    <testcase time="0" classname="Expectations.Failure:2a">
-      <failure type="failure" message="expectation_failure in structure(list(f(), fail()), start_frame = 57): Failure has been forced&#10;&#10; @reporters/tests.R#12"/>
+    <testcase time="0" classname="Expectations" name="Failure:2a">
+      <failure type="failure" message="Failure has been forced">Failure has been forced
+</failure>
     </testcase>
-    <testcase time="0" classname="Expectations.Failure:2b">
-      <failure type="failure" message="expectation_failure in structure(list(expect_true(FALSE)), start_frame = 57): FALSE isn't true.&#10;&#10; @reporters/tests.R#15"/>
+    <testcase time="0" classname="Expectations" name="Failure:2b">
+      <failure type="failure" message="FALSE isn't true.">FALSE isn't true.
+</failure>
     </testcase>
-    <testcase time="0" classname="Expectations.Failure:loop">
-      <failure type="failure" message="expectation_failure in structure(list(expect_equal(i, 2)), start_frame = 57): `i` not equal to 2.&#10;1/1 mismatches&#10;[1] 1 - 2 == -1&#10;&#10; @reporters/tests.R#20"/>
+    <testcase time="0" classname="Expectations" name="Failure:loop">
+      <failure type="failure" message="`i` not equal to 2.">`i` not equal to 2.
+1/1 mismatches
+[1] 1 - 2 == -1
+</failure>
     </testcase>
-    <testcase time="0" classname="Expectations.Failure:loop"/>
+    <testcase time="0" classname="Expectations" name="Failure:loop"/>
   </testsuite>
   <testsuite name="Errors" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="2" skipped="0" failures="0" errors="2" time="0">
-    <testcase time="0" classname="Errors.Error:1">
-      <error type="error" message="expectation_error in structure(list(stop(&quot;stop&quot;)), start_frame = 57): stop&#10; @reporters/tests.R#28"/>
+    <testcase time="0" classname="Errors" name="Error:1">
+      <error type="error" message="stop">stop
+1: stop("stop") at reporters/tests.R:28</error>
     </testcase>
-    <testcase time="0" classname="Errors.Error:3">
-      <error type="error" message="expectation_error in structure(list(f(), g(), h(), stop(&quot;!&quot;)), start_frame = 57): !&#10; @reporters/tests.R#36"/>
+    <testcase time="0" classname="Errors" name="Error:3">
+      <error type="error" message="!">!
+1: f() at reporters/tests.R:36
+2: g() at reporters/tests.R:32
+3: h() at reporters/tests.R:33
+4: stop("!") at reporters/tests.R:34</error>
     </testcase>
   </testsuite>
   <testsuite name="Recursion" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="1" skipped="0" failures="0" errors="1" time="0">
-    <testcase time="0" classname="Recursion.Recursion:1">
-      <error type="error" message="expectation_error in structure(list(f(), f(), f(), f(), f(), f(), f(), f(), f(), f(), : evaluation nested too deeply: infinite recursion / options(expressions=)?&#10; @reporters/tests.R#43"/>
+    <testcase time="0" classname="Recursion" name="Recursion:1">
+      <error type="error" message="evaluation nested too deeply: infinite recursion / options(expressions=)?">evaluation nested too deeply: infinite recursion / options(expressions=)?
+1: f() at reporters/tests.R:43
+2: f() at reporters/tests.R:42
+3: f() at reporters/tests.R:42
+4: f() at reporters/tests.R:42
+5: f() at reporters/tests.R:42
+6: f() at reporters/tests.R:42
+7: f() at reporters/tests.R:42
+8: f() at reporters/tests.R:42
+9: f() at reporters/tests.R:42
+10: 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</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">
-    <testcase time="0" classname="Skips.Skip:1">
+    <testcase time="0" classname="Skips" name="Skip:1">
       <skipped/>
     </testcase>
-    <testcase time="0" classname="Skips.Skip:2">
+    <testcase time="0" classname="Skips" name="Skip:2">
       <skipped/>
     </testcase>
-    <testcase time="0" classname="Skips.Skip:3">
+    <testcase time="0" classname="Skips" name="Skip:3">
       <skipped/>
     </testcase>
   </testsuite>
   <testsuite name="Warnings" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="3" skipped="0" failures="0" errors="0" time="0">
-    <testcase time="0" classname="Warnings.Warning:1"/>
-    <testcase time="0" classname="Warnings.Warning:2"/>
-    <testcase time="0" classname="Warnings.Warning:2"/>
+    <testcase time="0" classname="Warnings" name="Warning:1"/>
+    <testcase time="0" classname="Warnings" name="Warning:2"/>
+    <testcase time="0" classname="Warnings" name="Warning:2"/>
   </testsuite>
   <testsuite name="Output" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="2" skipped="0" failures="0" errors="0" time="0">
-    <testcase time="0" classname="Output.Output:1"/>
-    <testcase time="0" classname="Output.Output:1"/>
+    <testcase time="0" classname="Output" name="Output:1"/>
+    <testcase time="0" classname="Output" name="Output:1"/>
   </testsuite>
-</testsuites>
+</testsuites>