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


Reply via email to