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

expect_s3_class for class hierarchies #885

Closed
krlmlr opened this issue Jun 15, 2019 · 7 comments
Closed

expect_s3_class for class hierarchies #885

krlmlr opened this issue Jun 15, 2019 · 7 comments
Labels
expectation 🙀 feature a feature request or enhancement

Comments

@krlmlr
Copy link
Member

krlmlr commented Jun 15, 2019

What's a good way to test that an object inherits from multiple classes? expect_s3_class() uses inherits() which tests if the object inherits from one of the classes passed.

Can we add a new expect_s3_classes() ?

# This is expected
testthat::expect_s3_classes(tibble::tibble(), c("tbl_df", "data.frame"))
# This should be a failure?
testthat::expect_s3_classes(tibble::tibble(), c("tbl_df", "data.table"))

Created on 2019-06-15 by the reprex package (v0.3.0)

@jimhester
Copy link
Member

You can use inherits(which = TRUE) to test all classes, it returns the indexes of the matches, or 0 if there are no matches.

all(inherits(tibble::tibble(), c("tbl_df", "data.frame"), which = TRUE) > 0)
#> [1] TRUE
all(inherits(tibble::tibble(), c("tbl_df", "data.table"), which = TRUE) > 0)
#> [1] FALSE

Created on 2019-06-17 by the reprex package (v0.3.0)

@krlmlr
Copy link
Member Author

krlmlr commented Jun 17, 2019

Is it worthwhile to turn this pattern into a first-class expectation?

@jimhester
Copy link
Member

It might be worth seeing how many packages break if we just make this behavior the default for expect_s3_class(), it seems to me like testing for all the classes is a more useful default than testing for any of them, like it is currently.

@lionel-
Copy link
Member

lionel- commented Jun 17, 2019

You can also use rlang::inherits_all() and rlang::inherits_only() in combination with expect_true().

@Eluvias
Copy link

Eluvias commented Jun 22, 2019

Similar to #667

@hadley
Copy link
Member

hadley commented Jul 15, 2019

Maybe we could add an exact = TRUE argument?

@hadley hadley added expectation 🙀 feature a feature request or enhancement labels Jul 15, 2019
@hadley hadley closed this as completed in 004b285 Jul 16, 2019
@ha0ye
Copy link

ha0ye commented Nov 5, 2019

Adding a +1 that I think exact = TRUE should be a default, as I otherwise missed a typo in checking against a vector of classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expectation 🙀 feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants