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

Fixing 'UTC' did not equal 'Etc/UTC' failure on Mac #7526

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Aug 9, 2023

Pull Request Description

Mitigates #7505 and makes sure CI passes again on our x64 based Mac OS X machines.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the Enso style guides.
  • All code has been tested:
    • Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 9, 2023
@JaroslavTulach JaroslavTulach requested a review from mwu-tow August 9, 2023 09:22
@JaroslavTulach JaroslavTulach self-assigned this Aug 9, 2023
Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want the check to pass, I think you need to remove the second added line, I'm pretty sure it will fail.

Definitely good to fix CI to pass, as always failing on Mac is not good - so I'm For merging this PR.

However, I would like to not close #7505. I think we should investigate why one system returns Etc/UTC and other just UTC.

Apparently they seem to be different names for the same concept...

Still, I'd like us to ensure that the line I just suggested you to remove actually works in longer term:

loose_zone_equal time.zone Time_Zone.utc

As a quick fix for CI - this is fine, but for longer-term it would be worth to investigate why there is this timezone inconsistency and fix it.

As users will definitely expect the TZ returned here to == Time_Zone.utc.

test/Tests/src/Data/Time/Date_Time_Spec.enso Outdated Show resolved Hide resolved
Co-authored-by: Radosław Waśko <[email protected]>
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Aug 10, 2023

There are two more [FAILED tests in the Mac OS X engine CI run:

On my Mac the tests are skipped, but not on CI...

@JaroslavTulach
Copy link
Member Author

Having CI green is important. I guess I merge now. @mwu-tow, can you make Mac OS X CI required again? Otherwise we are likely going to regress soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants