Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reporter output control via "file" argument or options setting (#629) #635

Merged
merged 9 commits into from Oct 10, 2017
19 changes: 10 additions & 9 deletions NEWS.md
Expand Up @@ -51,8 +51,8 @@
warnings are now in magenta, and skips in blue.

* New default reporter `ReporterProgress` replaces the previous
`SummaryReporter`. It's a careful rethining of the default output that is
both more aesthetical pleasing and makes the most important information
`SummaryReporter`. It's a careful rethinking of the default output that is
both more aesthetically pleasing and makes the most important information
available upfront (#529).

* `test_dir()` (and hence `test_package()`, and `test_check()`) now unsets
Expand All @@ -61,7 +61,7 @@
* `expect_setequal()` compares two set (represented by vectors), ignoring
duplicates and differences in order (#528).

* `expect_output_file()` recieved a few tweaks. It now defaults to
* `expect_output_file()` received a few tweaks. It now defaults to
`update = TRUE` and no longer fails on first run. It gains a `print`
argument to automatically print the input (#627).

Expand Down Expand Up @@ -103,13 +103,14 @@
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.
* Reporters accept a ``"file"`` argument on initialization. If provided, reporters
will write the test results to the provided path rather than
standard output. This output destination can also be controlled with the option
`testthat.output_file` (#635, @nealrichardson).

* 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
includes both the `classname` attribute, which contains the testthat
context, and the `name` attribute, which contains the testthat
test name.

* The default summary reporter aborts testing as soon as the limit given by the
Expand Down Expand Up @@ -160,7 +161,7 @@
* `test_file()` now triggers a `gc()` after tests are run. This helps
to ensure that finalisers are run earlier (#535).

* New argument`encoding` in `test_file()` and `source_file()` (@hansharhoff, #550, hadley/devtools#1306)
* New argument `encoding` in `test_file()` and `source_file()` (@hansharhoff, #550, hadley/devtools#1306)

* Special regular expression characters are escaped when printing errors in
`expect_match()` (#522, @jimhester).
Expand Down
6 changes: 2 additions & 4 deletions R/reporter-junit.R
Expand Up @@ -121,10 +121,8 @@ JunitReporter <- R6::R6Class("JunitReporter", inherit = Reporter,
},

end_reporter = function() {
output_file <- getOption("testthat.junit.output_file")

if (!is.null(output_file)) {
xml2::write_xml(self$doc, output_file, format = TRUE)
if (is.character(self$out)) {
xml2::write_xml(self$doc, self$out, format = TRUE)
} else if (inherits(self$out, "connection")) {
file <- tempfile()
xml2::write_xml(self$doc, file, format = TRUE)
Expand Down
8 changes: 3 additions & 5 deletions R/reporter-progress.R
Expand Up @@ -35,9 +35,9 @@ ProgressReporter <- R6::R6Class("ProgressReporter", inherit = Reporter,

initialize = function(show_praise = TRUE,
max_failures = getOption("testthat.progress.max_fails", 10L),
min_time = 0.1
) {
super$initialize()
min_time = 0.1,
...) {
super$initialize(...)
self$max_fail <- max_failures
self$show_praise <- show_praise
self$min_time <- min_time
Expand Down Expand Up @@ -212,5 +212,3 @@ issue_summary <- function(x) {
format(x)
)
}


4 changes: 2 additions & 2 deletions R/reporter-silent.R
Expand Up @@ -14,8 +14,8 @@ SilentReporter <- R6::R6Class("SilentReporter", inherit = Reporter,
public = list(
.expectations = NULL,

initialize = function() {
super$initialize()
initialize = function(...) {
super$initialize(...)
self$.expectations <- Stack$new()
},

Expand Down
7 changes: 5 additions & 2 deletions R/reporter-summary.R
Expand Up @@ -25,8 +25,11 @@ SummaryReporter <- R6::R6Class("SummaryReporter", inherit = Reporter,
show_praise = TRUE,
omit_dots = FALSE,

initialize = function(show_praise = TRUE, omit_dots = getOption("testthat.summary.omit_dots"), max_reports = getOption("testthat.summary.max_reports", 15L)) {
super$initialize()
initialize = function(show_praise = TRUE,
omit_dots = getOption("testthat.summary.omit_dots"),
max_reports = getOption("testthat.summary.max_reports", 15L),
...) {
super$initialize(...)
self$failures <- Stack$new()
self$skips <- Stack$new()
self$warnings <- Stack$new()
Expand Down
4 changes: 2 additions & 2 deletions R/reporter-zzz.R
Expand Up @@ -82,7 +82,7 @@ find_reporter <- function(reporter) {
}
}

find_reporter_one <- function(reporter) {
find_reporter_one <- function(reporter, ...) {
stopifnot(is.character(reporter))

name <- reporter
Expand All @@ -93,5 +93,5 @@ find_reporter_one <- function(reporter) {
stop("Can not find test reporter ", reporter, call. = FALSE)
}

get(name)$new()
get(name)$new(...)
}
17 changes: 14 additions & 3 deletions R/reporter.R
Expand Up @@ -21,8 +21,12 @@ Reporter <- R6::R6Class("Reporter",

out = NULL,

initialize = function() {
self$out <- stdout()
initialize = function(file = getOption("testthat.output_file", stdout())) {
self$out <- file
if (is.character(self$out) && file.exists(self$out)) {
# If writing to a file, overwrite it if it exists
file.remove(self$out)
}
},

cat = function(..., file = NULL, sep = " ", fill = FALSE, labels = NULL,
Expand All @@ -34,7 +38,14 @@ Reporter <- R6::R6Class("Reporter",
warning("append ignored", call. = FALSE)
}

cat(..., file = self$out, sep = sep, fill = fill, labels = labels)
cat(
...,
file = self$out,
sep = sep,
fill = fill,
labels = labels,
append = is.character(self$out) # If writing to file, append=TRUE
)
},

cat_tight = function(...) {
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/helper-junitmock.R
Expand Up @@ -6,7 +6,7 @@
# - timestamp - originally wrapper for toString(Sys.time())
# - hostname - originally wrapper for Sys.info()[["nodename"]]
#
createJunitReporterMock <- function () {
createJunitReporterMock <- function (...) {
JunitReporterMock <- R6::R6Class("JunitReporterMock",
inherit = JunitReporter,
public = list (),
Expand All @@ -22,5 +22,5 @@ createJunitReporterMock <- function () {
}
)
)
JunitReporterMock$new()
JunitReporterMock$new(...)
}
120 changes: 116 additions & 4 deletions tests/testthat/test-reporter.R
Expand Up @@ -38,6 +38,14 @@ test_that("character vector yields multi reporter", {
)
})

test_reporter <- function(reporter) {
# Function to run the reporter "test suite" with a given reporter
withr::with_options(
list(expressions = Cstack_info()[["eval_depth"]] + 200),
test_file(test_path("reporters/tests.R"), reporter, wrap = FALSE)
)
}

test_that("reporters produce consistent output", {
old <- options(width = 80)
on.exit(options(old), add = TRUE)
Expand All @@ -47,10 +55,7 @@ test_that("reporters produce consistent output", {

expect_known_output(
expect_error(
withr::with_options(
list(expressions = Cstack_info()[["eval_depth"]] + 200),
test_file(test_path("reporters/tests.R"), reporter, wrap = FALSE)
),
test_reporter(reporter),
error_regexp
),
path, update = TRUE)
Expand All @@ -77,4 +82,111 @@ test_that("reporters produce consistent output", {
save_report("silent")
save_report("rstudio")
save_report("junit", reporter = createJunitReporterMock())

# Test that MultiReporter can write to two different places
tap_file <- tempfile()
save_report("summary", reporter = MultiReporter$new(list(
SummaryReporter$new(show_praise = FALSE, omit_dots = FALSE),
TapReporter$new(file=tap_file))
))
expect_identical(read_lines(tap_file),
read_lines(test_path("reporters", "tap.txt")))
})


test_report_to_file <- function(name, reporter = find_reporter_one(name, ...),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call this expect_...

output_file, error_regexp=NA, ...) {
# output_file is where we expect output to be written to, whether we pass it
# as an argument to Reporter$new() (here, via the ...), or whether it is set
# in an option.
path <- test_path("reporters", paste0(name, ".txt"))
expect_silent(
expect_error(
test_reporter(reporter),
error_regexp
)
)
expect_identical(read_lines(output_file), read_lines(path))
}

test_that("reporters accept a 'file' arugment and write to that location", {
output <- tempfile()
test_report_to_file("check", file = output, error_regexp=NULL, output_file = output)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You only need to test each reporter once, i.e. not multiple times for SummaryReporter

test_report_to_file(
"progress",
ProgressReporter$new(show_praise = FALSE, min_time = Inf, file = output),
output_file = output
)
test_report_to_file(
"summary",
SummaryReporter$new(show_praise = FALSE, omit_dots = FALSE, file = output),
output_file = output
)
test_report_to_file(
"summary-2",
SummaryReporter$new(show_praise = FALSE, max_reports = 2, file = output),
output_file = output
)
test_report_to_file(
"summary-no-dots",
SummaryReporter$new(show_praise = FALSE, omit_dots = TRUE, file = output),
output_file = output
)
test_report_to_file("location", file = output, output_file = output)
test_report_to_file("minimal", file = output, output_file = output)
test_report_to_file("tap", file = output, output_file = output)
test_report_to_file("teamcity", file = output, output_file = output)
test_report_to_file("rstudio", file = output, output_file = output)
test_report_to_file(
"junit",
reporter = createJunitReporterMock(file = output),
output_file = output
)
})

test_that("reporters write to 'testthat.output_file', if specified", {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this only needs one test since it's only one line of code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was in testing all of the reporters that I found that several of them needed ... added to their initialize methods to pass the file argument through to super$initialize, so there is actually more than just the one line of code exercised by these tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that applies to the test case above, not this one? (Because the option is set in the base class initialiser)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, misread. You're right, will prune.

output_option <- tempfile()
withr::with_options(list(testthat.output_file = output_option), {
test_report_to_file("check", error_regexp=NULL, output_file = output_option)
test_report_to_file(
"progress",
ProgressReporter$new(show_praise = FALSE, min_time = Inf),
output_file = output_option
)
test_report_to_file(
"summary",
SummaryReporter$new(show_praise = FALSE, omit_dots = FALSE),
output_file = output_option
)
test_report_to_file(
"summary-2",
SummaryReporter$new(show_praise = FALSE, max_reports = 2),
output_file = output_option
)
test_report_to_file(
"summary-no-dots",
SummaryReporter$new(show_praise = FALSE, omit_dots = TRUE),
output_file = output_option
)
test_report_to_file("location", output_file = output_option)
test_report_to_file("minimal", output_file = output_option)
test_report_to_file("tap", output_file = output_option)
test_report_to_file("teamcity", output_file = output_option)
test_report_to_file("rstudio", output_file = output_option)
test_report_to_file(
"junit",
reporter = createJunitReporterMock(),
output_file = output_option
)
})
})

test_that("silent reporter accepts the 'file' argument but doesn't write anything", {
output <- tempfile()
expect_silent(test_reporter(SilentReporter$new(file=output)))
expect_false(file.exists(output))
withr::with_options(list(testthat.output_file = output), {
expect_silent(test_reporter(SilentReporter$new()))
expect_false(file.exists(output))
})
})