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

[R] strptime tests not robust across platforms #32658

Closed
asfimport opened this issue Aug 11, 2022 · 3 comments
Closed

[R] strptime tests not robust across platforms #32658

asfimport opened this issue Aug 11, 2022 · 3 comments
Assignees
Milestone

Comments

@asfimport
Copy link
Collaborator

After the 9.0.0 release was accepted on CRAN, Ripley emailed me about a test failure on some other machine, which has not yet shown up on CRAN checks:

── Failure (test-dplyr-funcs-datetime.R:183:5): strptime ───────────────────────
  `object` (`actual`) not equal to `expected` (`expected`).
  
  actual vs expected
                                      x
  - actual[1, ]                      NA
  + expected[1, ]   1999-03-16 12:22:20
  - actual[2, ]                      NA
  + expected[2, ]   1999-10-08 18:02:24
  - actual[3, ]                      NA
  + expected[3, ]   1999-04-04 03:52:27
  - actual[4, ]                      NA
  + expected[4, ]   1999-05-28 11:35:45
  - actual[5, ]                      NA
  + expected[5, ]   1999-03-16 08:08:55
  - actual[6, ]                      NA
  + expected[6, ]   1999-09-25 00:19:59
  - actual[7, ]                      NA
  + expected[7, ]   1999-10-12 20:47:55
  - actual[8, ]                      NA
  + expected[8, ]   1999-04-15 20:36:12
  - actual[9, ]                      NA
  + expected[9, ]   1999-05-01 03:55:23
  - actual[10, ]                     NA
  + expected[10, ]  1999-12-15 01:19:05
  and 90 more ...
  
       actual$x | expected$x                           
   [1] NA       - "1999-03-16 12:22:20" [1]            
   [2] NA       - "1999-10-08 18:02:24" [2]            
   [3] NA       - "1999-04-04 03:52:27" [3]            
   [4] NA       - "1999-05-28 11:35:45" [4]            
   [5] NA       - "1999-03-16 08:08:55" [5]            
   [6] NA       - "1999-09-25 00:19:59" [6]            
   [7] NA       - "1999-10-12 20:47:55" [7]            
   [8] NA       - "1999-04-15 20:36:12" [8]            
   [9] NA       - "1999-05-01 03:55:23" [9]            
  [10] NA       - "1999-12-15 01:19:05" [10]           
   ... ...        ...                   and 90 more ...
  Backtrace:
      ▆
   1. └─arrow:::expect_equal(...) at test-dplyr-funcs-datetime.R:183:4
   2.   └─testthat::expect_equal(...) at tests/testthat/helper-expectation.R:42:4
  
  [ FAIL 1 | WARN 0 | SKIP 79 | PASS 8173 ]

It appears that one of the strptime tests returns NA in Arrow but not in R. Reading the test, it uses R to first strftime and then tests that Arrow and R both strptime that back, so it could be an R quirk: R recognizes and can do something with this strptime token round trip, but our library doesn't.

Unfortunately, I don't know which token it is though because these tests are run in a for loop and the failure message doesn't say which token is the one that is failing. testthat does provide some facilities for reporting useful things within a loop, so we should wire those up.

In addition to better handling of tests in a loop, we should probably just skip this whole thing on CRAN.

Reporter: Neal Richardson / @nealrichardson
Assignee: Rok Mihevc / @rok
Watchers: Rok Mihevc / @rok

PRs and other links:

Note: This issue was originally created as ARROW-17386. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Neal Richardson / @nealrichardson:
We also might want to be explicit about which ones we expect to be all NA and which ones should be valid. My guess from reading the strptime docs is that on every other platform, this particular token is also invalid in R, but it is valid on BDR's machine.

@asfimport
Copy link
Collaborator Author

Rok Mihevc / @rok:
It's either strftime or strptime working in an unexpected way. We should rewrite the test to be more explicit and only test strptime (like we do parse_date_time: https://github.com/apache/arrow/blob/master/r/tests/testthat/test-dplyr-funcs-datetime.R#L2382-L2400).

@asfimport
Copy link
Collaborator Author

Rok Mihevc / @rok:
Issue resolved by pull request 13854
#13854

@raulcd raulcd modified the milestones: 9.0.1, 10.0.0 Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants