[GitHub] [arrow] thisisnic commented on a change in pull request #11417: ARROW-13799 [R] case_when error handling is capturing strings

[GitBox <gi...@apache.org>]

thisisnic commented on a change in pull request #11417:
URL: https://github.com/apache/arrow/pull/11417#discussion_r728835299



##########
File path: r/tests/testthat/test-dplyr-funcs-conditional.R
##########
@@ -268,14 +268,15 @@ test_that("case_when()", {
       collect(),
     tbl
   )
-  skip("ARROW-13799: factor() should error but instead we get a string error message in its place")
+  # skip("ARROW-13799: factor() should error but instead we get a string error message in its place")
   expect_dplyr_equal(
     input %>%
       mutate(
         cw = case_when(!(!(!(lgl))) ~ factor(chr), TRUE ~ fct)
       ) %>%
       collect(),
-    tbl
+    tbl,
+    warning = "case_when"

Review comment:
       What warning message is produced here? Could you use warning = `TRUE` or is that not applicable in this case? (Genuinely asking, haven't run the code!)

##########
File path: r/tests/testthat/test-dplyr-funcs-conditional.R
##########
@@ -268,14 +268,15 @@ test_that("case_when()", {
       collect(),
     tbl
   )
-  skip("ARROW-13799: factor() should error but instead we get a string error message in its place")
+  # skip("ARROW-13799: factor() should error but instead we get a string error message in its place")

Review comment:
       Instead of commenting out the skip message, please could you remove it entirely, create a JIRA for implementing the functionality (if one doesn't exist already - may need to search), and then add a TODO above in a comment referencing the JIRA.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org