Skip to content

Conversation

@wgtmac
Copy link
Member

@wgtmac wgtmac commented Mar 21, 2024

What changes were proposed in this pull request?

Enable TestTimezone.testMissingTZDB unit test to run on Windows.

Why are the changes needed?

When /usr/share/zoneinfo is unavailable and TZDIR env is unset, creating C++ ORC reader will crash on Windows. We need to better deal with this case. See context from the Apache Arrow community: apache/arrow#36026 and apache/arrow#40633

How was this patch tested?

Make sure the test passes on Windows.

Was this patch authored or co-authored using generative AI tooling?

No.

@wgtmac wgtmac marked this pull request as draft March 21, 2024 06:22
@github-actions github-actions bot added the CPP label Mar 21, 2024
@wgtmac wgtmac changed the title WIP: Enable TestTimezone.testMissingTZDB on Windows ORC-1663: [C++] Enable TestTimezone.testMissingTZDB on Windows Mar 21, 2024
@wgtmac wgtmac marked this pull request as ready for review March 21, 2024 16:02
@dongjoon-hyun dongjoon-hyun modified the milestones: 2.1.0, 2.0.1 Mar 22, 2024
dongjoon-hyun pushed a commit that referenced this pull request Mar 22, 2024
### What changes were proposed in this pull request?

Enable TestTimezone.testMissingTZDB unit test to run on Windows.

### Why are the changes needed?

When /usr/share/zoneinfo is unavailable and TZDIR env is unset, creating C++ ORC reader will crash on Windows. We need to better deal with this case. See context from the Apache Arrow community: apache/arrow#36026 and apache/arrow#40633

### How was this patch tested?

Make sure the test passes on Windows.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #1856 from wgtmac/win_tz_test.

Authored-by: Gang Wu <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Merged to main/2.0.

@wgtmac
Copy link
Member Author

wgtmac commented Mar 23, 2024

Thanks @dongjoon-hyun !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants