jonkeane commented on a change in pull request #11250: URL: https://github.com/apache/arrow/pull/11250#discussion_r716826516
########## File path: r/tests/testthat/test-dplyr-string-functions.R ########## @@ -806,26 +806,49 @@ test_that("format_ISO8601", { times ) - expect_dplyr_equal( - input %>% - mutate(x = format_ISO8601(x, precision = "ymd", usetz = TRUE)) %>% - collect(), - times - ) + if (getRversion() < "3.5") { + # before 3.5, times$x will have no timezone attribute, so Arrow faithfully + # errors that there is no timezone to format: + expect_error( + times %>% + Table$create() %>% + mutate(x = format_ISO8601(x, precision = "ymd", usetz = TRUE)) %>% + collect(), + "Timezone not present, cannot convert to string with timezone: %Y-%m-%d%z" + ) + + # See comment regarding %S flag in strftime tests + expect_error( + times %>% + Table$create() %>% + mutate(x = format_ISO8601(x, precision = "ymdhms", usetz = TRUE)) %>% + mutate(x = gsub("\\.0*", "", x)) %>% + collect(), + "Timezone not present, cannot convert to string with timezone: %Y-%m-%dT%H:%M:%S%z" + ) + } else { + expect_dplyr_equal( + input %>% + mutate(x = format_ISO8601(x, precision = "ymd", usetz = TRUE)) %>% + collect(), + times + ) + + # See comment regarding %S flag in strftime tests + expect_dplyr_equal( + input %>% + mutate(x = format_ISO8601(x, precision = "ymdhms", usetz = TRUE)) %>% + mutate(x = gsub("\\.0*", "", x)) %>% + collect(), + times + ) + } - # See comment regarding %S flag in strftime tests - expect_dplyr_equal( - input %>% - mutate(x = format_ISO8601(x, precision = "ymdhms", usetz = FALSE)) %>% - mutate(x = gsub("\\.0*", "", x)) %>% - collect(), - times - ) # See comment regarding %S flag in strftime tests expect_dplyr_equal( input %>% - mutate(x = format_ISO8601(x, precision = "ymdhms", usetz = TRUE)) %>% + mutate(x = format_ISO8601(x, precision = "ymdhms", usetz = FALSE)) %>% Review comment: This diff and the one above it make this a bit confusing. What I actually did was moved the `usetz = FALSE` block up above into the if/else gated on R version. But git took that to mean I deleted the `usetz = FALSE` and then changed the `FALSE` to `TRUE` here. Both cases are still in this file, and for anything R 3.5 and higher we test that it works, for < 3.5 we test the error -- 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-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org