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

Add expect_predicate() for chainable inspection? #1338

Closed
Aehmlo opened this issue Feb 25, 2021 · 1 comment
Closed

Add expect_predicate() for chainable inspection? #1338

Aehmlo opened this issue Feb 25, 2021 · 1 comment

Comments

@Aehmlo
Copy link

Aehmlo commented Feb 25, 2021

In the course of writing up this issue, I've realized that magrittr's "tee" pipe is a good candidate for this, but since testthat doesn't re-export this function (nor am I convinced that it necessarily should), I figured I'd float this idea anyway.

In the unit tests for a function I'm working on, I want to ensure that an object is an array, along with some other assertions. To chain these assertions, I used the pipe operator, as alluded to by testthat's documentation. It looked something like the below.

f(x) %>%
  {expect_true(is.array(.))} %>%
  custom_expectation()

When running the test, my custom expectation kept failing, which was bewildering, since it definitely held in interactive inspection. Eventually, it dawned on me that the custom expectation was being passed is.array(f(x)) instead of f(x) itself. It's perfectly clear to me now why this is the case, but I spent some time very confused.

To help overcome this, I'm wondering if it might be suitable to introduce an expectation, something like expect_predicate(), which could evaluate a predicate for truth, returning the original object instead of TRUE/FALSE. This would enable chains where multiple orthogonal object properties are investigated while retaining the use of the "normal" pipe %>%:

f(x) %>%
  expect_predicate(!is.null(.$a)) %>%
  expect_predicate(.$b > 0) %>%
  expect_class("f")

I believe the second form above might require some metaprogramming to pull off properly.

If this is something that's deemed potentially of interest, I'd be happy to write up a PR (and/or proof-of-concept for trying it out), but I figured I'd open this issue before doing so.

@hadley
Copy link
Member

hadley commented Jul 7, 2021

I agree with your definition of the problem; this is something that bothers me too. But I haven't yet been able to come up with an approach that I'm happy with. The problem with expect_predicate() is that you'd want want similar variants for expect_equal(), expect_s3_class(), etc, so the total number of functions would be quite high.

I also played around a bit with a tee() function:

tee <- function(x, ...) {
  list(...)
  x
}

f(x) %>%
  tee(expect_true(is.array(.))) %>%
  custom_expectation()

But ultimately I couldn't persuade myself it was useful/general enough.

@hadley hadley closed this as completed Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants