Skip to content

Conversation

@tisba
Copy link
Collaborator

@tisba tisba commented Jul 2, 2025

I saw @bnoordhuis's change in #352.

I think instead of swallowing the error because it breaks in CI, we should fix the CI setup. I suspect that the TZInfo zoneinfo data is simply missing in the Ruby alpine docker image.

I tested this with docker and ruby:3.4-alpine. When I install the required tzdata (via apk add --no-cache tzdata) it works as expected:

irb(main):001> require "active_support"
=> true
irb(main):002> require "active_support/time"
=> true
irb(main):003> Time.zone = "UTC"
=> "UTC"

Without the tzdatapackage, it breaks like described in #351.

@tisba tisba force-pushed the fix/install-tzdata branch from 97f1ab2 to 4724bca Compare July 2, 2025 17:44
@bnoordhuis
Copy link
Collaborator

I think instead of swallowing the error because it breaks in CI, we should fix the CI setup.

That's not the sole reason. It's also for people running tests locally.

Speaking from experience with node and libuv: test suites shouldn't come with too many constraints if you want people to actually run them.

@tisba
Copy link
Collaborator Author

tisba commented Jul 3, 2025

oh sorry, that's actually not what I meant to say 🤦 I do think we should keep the change. Meant to say that we should setup CI in a way that the test is not skipped.

That's why I also changed the mini test output in CI, so we actually see tests that have been skipped.

@tisba tisba requested a review from bnoordhuis July 3, 2025 08:02
@tisba tisba merged commit 99a166d into main Jul 3, 2025
27 of 32 checks passed
@tisba tisba deleted the fix/install-tzdata branch July 3, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants