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

Update test for problems() unwind::protect error #445

Closed
sbearrows opened this issue Jun 3, 2022 · 1 comment · Fixed by #452
Closed

Update test for problems() unwind::protect error #445

sbearrows opened this issue Jun 3, 2022 · 1 comment · Fixed by #452
Assignees

Comments

@sbearrows
Copy link
Contributor

The test in question:

# https://github.com/tidyverse/vroom/pull/441#discussion_r883611090
test_that("can promote vroom parse warning to error", {
make_warning <- function() {
x <- vroom(
I("a,b\n1.0,x\n"),
delim = ",",
col_types = "dd"
)
# Warning happens at print time so force a print
print(x)
}
expect_error(
# This fails hard if we unwind protect the warning (aborts RStudio)
# - Try to throw error after catching the warning
withCallingHandlers(
expr = make_warning(),
vroom_parse_issue = function(cnd) {
rlang::abort("oh no")
}
)
)
})

In a PR to update the warning message from problems() we attempted to use cpp11::package() to call cli::cli_warn(). For a number of reasons this isn't a great idea and can actually crash R #441 (comment)

At the time we wrote the test, we didn't fully understand the circumstances that caused the crash. Now that we are more aware of how it occurs, we should update the test to reflect this information.

@DavisVaughan
Copy link
Member

See r-lib/cpp11#274 for full details of the crash

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

Successfully merging a pull request may close this issue.

2 participants