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
Copy path View file
12 NEWS.md
@@ -21,8 +21,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
@@ -31,7 +31,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).

@@ -78,8 +78,8 @@
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
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
@@ -133,7 +133,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).
Copy path View file
@@ -121,10 +121,10 @@ JunitReporter <- R6::R6Class("JunitReporter", inherit = Reporter,
},

end_reporter = function() {
output_file <- getOption("testthat.junit.output_file")
self$out <- getOption("testthat.junit.output_file", self$out)

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)
Copy path View file
@@ -38,9 +38,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_failures <- max_failures
self$show_praise <- show_praise
self$min_time <- min_time
@@ -205,5 +205,3 @@ issue_summary <- function(x) {
format(x)
)
}


Copy path View file
@@ -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()
},

Copy path View file
@@ -25,8 +25,8 @@ 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()
Copy path View file
@@ -82,7 +82,7 @@ find_reporter <- function(reporter) {
}
}

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

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

get(name)$new()
get(name)$new(...)
}
Copy path View file
@@ -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,
@@ -34,7 +38,8 @@ 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,

This comment has been minimized.

Copy link
@hadley

hadley Oct 10, 2017

Member

Can you please use tidyverse style here? http://style.tidyverse.org/syntax.html#long-lines

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

cat_tight = function(...) {
@@ -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 (),
@@ -22,5 +22,5 @@ createJunitReporterMock <- function () {
}
)
)
JunitReporterMock$new()
JunitReporterMock$new(...)
}
Copy path View file
@@ -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)
@@ -47,10 +55,7 @@ test_that("reporters produce consistent output", {

expect_output_file(
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)
@@ -77,4 +82,71 @@ 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, ...),

This comment has been minimized.

Copy link
@hadley

hadley Oct 10, 2017

Member

I'd call this expect_...

output_file, ...) {
# 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(test_reporter(reporter))
expect_identical(read_lines(output_file), read_lines(path))
}

output <- tempfile()

This comment has been minimized.

Copy link
@hadley

hadley Oct 10, 2017

Member

Could you mimic the approach of the "reporters produce consistent output" test? I think it's better to have one test, with some copy and paste inside of it. It's too hard to eliminate all the repetition here because of the varying arguments to the reporter.

output_option <- tempfile()
for (r in c("location", "minimal", "tap", "teamcity", "rstudio")) {
test_that(paste(r, "reporter accepts a 'file' on initialization"), {
test_report_to_file(r, output_file = output, file = output)
})
withr::with_options(list(testthat.output_file = output_option), {
test_that(paste(r, "reporter uses testthat.output_file, if specified"), {
test_report_to_file(r, output_file = output_option)
})
})
}

withr::with_options(list(testthat.summary.omit_dots = FALSE), {
for (r in c("summary", "progress")) {
# Do these separately to force show_praise = FALSE
test_that(paste(r, "reporter accepts a 'file' on initialization"), {
test_report_to_file(r, output_file = output, file = output,
show_praise = FALSE)
})
withr::with_options(list(testthat.output_file = output_option), {
test_that(paste(r, "reporter uses testthat.output_file, if specified"), {
test_report_to_file(r, output_file = output_option,
show_praise = FALSE)
})
})
}
})

junit_output_option <- tempfile()
test_that(paste("junit reporter accepts a 'file' on initialization"), {
test_report_to_file("junit", createJunitReporterMock(file = output),
output_file = output)
})
withr::with_options(list(testthat.output_file = output_option), {
test_that("junit uses testthat.output_file", {
test_report_to_file("junit", createJunitReporterMock(),
output_file = output_option)
})
withr::with_options(list(testthat.junit.output_file = junit_output_option), {
test_that("testthat.junit.output_file overrides", {
test_report_to_file("junit", createJunitReporterMock(),
output_file = junit_output_option)
})
})
})
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.