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

Add code to test test_that calls. #83

Merged
merged 2 commits into from Jul 19, 2013

Conversation

Projects
None yet
2 participants
@craigcitro
Contributor

craigcitro commented Jul 3, 2013

I wanted to make test-xxx.r actually testable, which was complicated by the fact that test_that is designed not to return anything. The natural fit seemed to be to pass in a reporter that did my bidding, which is what I've done. I think the result is that test_test_that works fairly reasonably, but if you think it's gotten too fancy for its own good, that's okay, too.

(Note that this branch is based on the other testing branch, so the code will look repetitive -- only the last diff is new. Is there a way to tell github to only show the relative changes? I'll clean this up once the other pull request is in.)

if (result$passed) return()
new_failure <- list()
new_failure[[test]] = result
failures <<- c(failures, new_failure)

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

I think those three lines are equivalent to failures[[test]] <<- new_failure

This comment has been minimized.

@craigcitro

craigcitro Jul 18, 2013

Contributor

touche. (originally there was a browser() call before the last line while debugging ...)

#' @include reporter.r
NULL
#' Test reporter: gather all errors silently.

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

Nice idea!

@@ -33,7 +33,7 @@ StopReporter <- setRefClass("StopReporter", contains = "Reporter",
test <<- NULL
if (length(failures) == 0) return()
messages <- vapply(failures, "[[", "", "message")
messages <- vapply(failures, as.character, "")

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

Can you use character(1) instead of ""

@@ -50,8 +62,12 @@ watch <- function(path, callback, pattern = NULL, hash = TRUE) {
dir_state <- function(path, pattern = NULL, hash = TRUE) {
files <- dir(path, pattern, full.names = TRUE)
# It's possible for any of the files to be deleted between the dir()

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

Nice, thanks!

set_reporter(old_reporter)
test_that(desc, {
if (failure_expected) {
info = 'Test succeeded when failure expected'

This comment has been minimized.

@hadley

This comment has been minimized.

@craigcitro

craigcitro Jul 18, 2013

Contributor

Bad fingers! Got the one below, too.

fail()
})
test_test_that("true is not false", {
expect_that(TRUE, is_false())

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

Copy and pasto?

This comment has been minimized.

@hadley

hadley Jul 18, 2013

Member

Oh, no it's just that the ordering has changed.

@hadley

This comment has been minimized.

Member

hadley commented Jul 18, 2013

Looking good. Can you rebase (once I've merged the other pull request) and squash? (And add a similar bullet to NEWS)

craigcitro added some commits Jun 26, 2013

Remove a race condition in `watch`.
Currently, `dir_state` can fail in unfortuate ways when a file disappears
between reading a directory listing and calculating a hash of its contents;
this patch fixes the problem by catching the failure and returning NA.
Add code to test `test_that`.
This change adds a new reporter that simply gathers expectation failures
silently, and rewrites `inst/tests/test-xxx.r` to use it.
@craigcitro

This comment has been minimized.

Contributor

craigcitro commented Jul 18, 2013

This is ready to go modulo merging the other pull request -- it's based on that one, so presumably it "just works" once the first is merged.

There was a little excitement on my side when I accidentally used rebase --onto -- I think everything's fine now.

hadley added a commit that referenced this pull request Jul 19, 2013

Merge pull request #83 from craigcitro/testtest
Add code to test test_that calls.

@hadley hadley merged commit a074ffc into r-lib:master Jul 19, 2013

@hadley

This comment has been minimized.

Member

hadley commented Jul 19, 2013

Thanks again for all this work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment