Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export(expect)
export(expect_condition)
export(expect_contains)
export(expect_cpp_tests_pass)
export(expect_disjoint)
export(expect_equal)
export(expect_equal_to_reference)
export(expect_equivalent)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# testthat (development version)

* New `expect_disjoint()` to check for the absence of values (#1851).
* New `vignette("mocking")` explains mocking in detail (#1265).
* New `vignette("challenging-functions")` provides an index to other documentation organised by testing challenges (#1265).
* When running a test interactively, testthat now reports the number of succeses. The results should also be more useful if you are using nested tests.
Expand Down
32 changes: 31 additions & 1 deletion R/expect-setequal.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
#' and that every element of `y` occurs in `x`.
#' * `expect_contains(x, y)` tests that `x` contains every element of `y`
#' (i.e. `y` is a subset of `x`).
#' * `expect_in(x, y)` tests every element of `x` is in `y`
#' * `expect_in(x, y)` tests that every element of `x` is in `y`
#' (i.e. `x` is a subset of `y`).
#' * `expect_disjoint(x, y)` tests that no element of `x` is in `y`
#' (i.e. `x` is disjoint from `y`).
#' * `expect_mapequal(x, y)` treats lists as if they are mappings between names
#' and values. Concretely, this drops `NULL`s in both objects and sorts
#' named components.
Expand Down Expand Up @@ -117,6 +119,7 @@ expect_contains <- function(object, expected) {
pass(act$val)
}


#' @export
#' @rdname expect_setequal
expect_in <- function(object, expected) {
Expand Down Expand Up @@ -144,6 +147,33 @@ expect_in <- function(object, expected) {
pass(act$val)
}

#' @export
#' @rdname expect_setequal
expect_disjoint <- function(object, expected) {
act <- quasi_label(enquo(object))
exp <- quasi_label(enquo(expected))

check_vector(object)
check_vector(expected)

act_common <- act$val %in% exp$val
if (any(act_common)) {
msg_exp <- sprintf(
"Expected %s to be disjoint from %s.",
act$lab,
exp$lab
)
msg_act <- c(
sprintf("Actual: %s", values(act$val)),
sprintf("Expected: none of %s", values(exp$val)),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sprintf("Expected: none of %s", values(exp$val)),
sprintf("Expected: None of %s", values(exp$val)),

I think I like having this capitalized more

sprintf("Invalid: %s", values(act$val[act_common]))
)
fail(c(msg_exp, msg_act))
}

pass(act$val)
}

# Helpers ----------------------------------------------------------------------

check_vector <- function(
Expand Down
7 changes: 6 additions & 1 deletion man/expect_setequal.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions tests/testthat/_snaps/expect-setequal.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,25 @@
Expected: "d", "e"
Invalid: "a", "b"

# expect_disjoint() gives useful message on failure

Code
expect_disjoint(x1, x2)
Condition
Error:
! Expected `x1` to be disjoint from `x2`.
Actual: "a", "b", "c"
Expected: none of "c", "d"
Invalid: "c"

---

Code
expect_disjoint(x1, x3)
Condition
Error:
! Expected `x1` to be disjoint from `x3`.
Actual: "a", "b", "c"
Expected: none of "b", "c", "d"
Invalid: "b", "c"

17 changes: 17 additions & 0 deletions tests/testthat/test-expect-setequal.R
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,20 @@ test_that("expect_in() gives useful message on failure", {
expect_snapshot_failure(expect_in(x1, x2))
expect_snapshot_failure(expect_in(x1, x3))
})

# disjoint ----------------------------------------------------------------

test_that("expect_disjoint() succeeds when appropriate", {
expect_success(expect_disjoint(1, letters))
expect_success(expect_disjoint(LETTERS, letters))
expect_success(expect_disjoint(character(), letters))
})

test_that("expect_disjoint() gives useful message on failure", {
x1 <- c("a", "b", "c")
x2 <- c("c", "d")
x3 <- c("b", "c", "d")

expect_snapshot_failure(expect_disjoint(x1, x2))
expect_snapshot_failure(expect_disjoint(x1, x3))
})
Copy link
Member

Choose a reason for hiding this comment

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

Might be useful to have a test for expect_disjoint(c("a", NA), NA) to test that missing values are matched exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. I made the requested changes. In doing so, I might have stumbled onto something else: expect_failure() does not succeed for expect_disjoint() and some other functions in this file, e.g.:

expect_failure(expect_in(3, 5))
## Error: Expected zero successes.
## Actually succeeded 1 times

I think that the reason is that the call of fail() is not inside return() for some functions, such that the later pass() is also executed. expect_snapshot_failure() seems to be ok with this but not expect_failure().

I could fix those missing return()s, but I'm not sure that it is good to mix this into this PR that is about something else.

Copy link
Member

Choose a reason for hiding this comment

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

@stibu81 that's because the expectation style has changed since you started working on this PR 😬 I've updated your expectation to the new style and expect_failure(expect_in(3, 5)) now correctly passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems I picked a bad moment for this... 😆 Thanks for fixing it.