GH-50041: [CI][Python] Make test_string_to_tzinfo_pytz_fallback more robust for platforms supporting lower case tz names#50042
Conversation
… more robust for platforms supporting lower case tz names
|
|
|
@github-actions crossbow submit -g verify-rc-source |
There was a problem hiding this comment.
Pull request overview
Fixes a flaky test by dynamically skipping test_string_to_tzinfo_pytz_fallback on platforms where zoneinfo recognizes lower-case timezone names (e.g., Windows in some configurations), instead of only skipping on macOS.
Changes:
- Removed the static
sys.platform == 'darwin'skip marker. - Added a runtime probe that attempts
zoneinfo.ZoneInfo("europe/brussels")and skips the test if it succeeds.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Revision: 3679dfb Submitted crossbow builds: ursacomputing/crossbow @ actions-0d37d07d55
|
|
@raulcd the verify-rc-source-windows is failing here, but for other reasons? (and I assume the python tests have not yet run there, to be able to validate this PR fixes the failure) |
Yes, the failing test has not run yet. I am unsure why but it is timing out. I am re-running just to validate it wasn't a transient issue. If it fails again we can temporarily increase the timeout to validate test passes and I can investigate the timing issues independently. |
|
This was successful on retry, I am merging |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 50eb001. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Rationale for this change
Fix failing test on CI as reported in #50041, for a test added in #49694
What changes are included in this PR?
The test relies on the lower case "europe/brussels" time zone name not being recognized by
zoneinfo(and it generally is bypytz). But apparently on some platforms,zoneinfodoes accept this. Updating the test to first test this, and thus skip the test dynamically.