-
Notifications
You must be signed in to change notification settings - Fork 318
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
Collect more reliable trace_env #2012
Conversation
And create more helpers for testing various more complicated success and failure modes. Fixes #1994
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 don't really know when I'd ever use these in my own packages, but I am on board with the symmetry since you already have expect_success()
and expect_failure()
expect_no_failure <- function(expr) { | ||
exp <- capture_failure(expr) |
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.
Maybe add a test? (i see you do have one for expect_no_success()
, and existing ones for expect_snapshot_failure()
)
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.
Like expect_failure(expect_no_failure(expect_true(FALSE)))
? 😆
Co-authored-by: Davis Vaughan <[email protected]>
And create more helpers for testing various more complicated success and failure modes.
Fixes #1994
@DavisVaughan no need to go deep here on the actual implementation/fix, but I'd appreciate your eyes on the newly exported expectation test helpers.