[GitHub] [arrow] jonkeane commented on a change in pull request #12240: ARROW-14442: [R] fix behaviour when converting timestamps with "" as tzone

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

jonkeane commented on a change in pull request #12240:
URL: https://github.com/apache/arrow/pull/12240#discussion_r840730171



##########
File path: r/tests/testthat/test-Array.R
##########
@@ -260,18 +260,30 @@ test_that("array supports POSIXct (ARROW-3340)", {
   expect_array_roundtrip(times2, timestamp("us", "US/Eastern"))
 })
 
-test_that("array supports POSIXct without timezone", {
-  # Make sure timezone is not set
+test_that("array uses local timezone for POSIXct without timezone", {
   withr::with_envvar(c(TZ = ""), {

Review comment:
       That is strange! The docs for withr don't _explicitly_ say this, but looking at the code it's "just" wrapping `with_envvar()` so `NAs` and `""` should both work there.
   
   You should submit an issue to withr about this and see what they say. We can merge this without using it in these places, it's just a little less clean + might not be testing what we think depending on the sequence of the tests being run (e.g. if the cache is set before we get here, the `with_envvar()` isn't actually changing the timezone...




-- 
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