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.

Sign up for GitHub

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

Jump to bottom

Update usage of expect_error() #263

Merged
merged 13 commits into from
Oct 22, 2024
Merged

Update usage of expect_error() #263

merged 13 commits into from
Oct 22, 2024

Conversation

hfrick
Copy link
Member

@hfrick hfrick commented Oct 21, 2024 •
edited
Loading

If it's testing for no error, use expect_no_condition() (default) or expect_no_error() (only if we get the empty dplyr condition bubbling up).

If it's testing the error message, use expect_snapshot(error = TRUE).

@hfrick hfrick requested a review from topepo October 21, 2024 13:02
@hfrick
Copy link
Member Author

hfrick commented Oct 21, 2024

Note to self / TODO: go with expect_no_error() over expect_no_condition() based on feedback in the tidyverse meeting.

topepo
topepo approved these changes Oct 21, 2024
View reviewed changes
Copy link
Member

@topepo topepo left a comment

Choose a reason for hiding this comment

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

Looks good.

Apart form this PR you might think about dropping some nocov's in there, esp the extract file.

@hfrick hfrick mentioned this pull request Oct 22, 2024
Closed
@hfrick hfrick merged commit 92ad391 into main Oct 22, 2024
11 checks passed
@hfrick hfrick deleted the expect_error branch October 22, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@topepo topepo topepo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hfrick @topepo