-
Notifications
You must be signed in to change notification settings - Fork 420
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
Update standalone files + standardize snapshot tests #1579
Conversation
…or()` was used. Put cnd_class on the first line to make it easy to remove it as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A few things to tweak across the board but otherwise looks great
tests/testthat/test-unnest.R
Outdated
expect_snapshot((expect_error(unnest(df, x)))) | ||
expect_snapshot(unnest(df, x), error = TRUE, cnd_class = TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at the updated snapshots. In all cases I believe we can drop cnd_class = TRUE
.
Our advice lately is that if you really care about the error class, then you do
testthat::expect_error(class = )
and if you also care about the error message, you do a separate
testthat::expect_snapshot(error = )
But lately we have not been combining both features into a single snapshot
And in all cases here in tidyr I don't think we care about the error class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in b49c272
Looks awesome thanks so much! |
usethis::use_standalone("r-lib/rlang")
exect_snapshot(error = TRUE)
for each errorexpect_error()
inside snapshots as we no longer need the error classexpect_warning(code, regexp = NA)
byexpect_no_warning()
testthat::set_max_fails(Inf)
for faster iteration during development