Skip to content

Conversation

@denini08
Copy link
Contributor

This PR fixes the issue reported in #1260 where tests failed on Windows because the "Europe/Berlin" timezone could not be loaded.

On Linux, zoneinfo can read timezone data from the system, but on Windows it requires the PyPI tzdata package.

@Copilot Copilot AI review requested due to automatic review settings August 15, 2025 23:49
@denini08 denini08 requested a review from a team as a code owner August 15, 2025 23:49
@denini08 denini08 requested review from ela-kotulska-frequenz and removed request for a team August 15, 2025 23:49
@github-actions github-actions bot added the part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) label Aug 15, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a Windows-specific test failure by adding the tzdata package dependency. The issue occurred because zoneinfo requires timezone data that is available by default on Linux but must be installed separately on Windows.

  • Adds tzdata package as a Windows-specific dependency for test environments
  • Resolves timezone loading issues when running tests on Windows platforms

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@llucax
Copy link
Contributor

llucax commented Aug 20, 2025

Hi @denini08, thanks for submitting a PR! 🙏

I'm not sure what to do about this, I will reply in the issue because there is nothing wrong with the PR itself, but I'm still unsure if we really want to solve this.

@github-actions github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Aug 21, 2025
@llucax
Copy link
Contributor

llucax commented Aug 23, 2025

Hi, thanks for the follow-up!

Tests are failing. Please make sure to run nox locally and make sure all checks pass.

Also ideally squash both commits when things are working again.

@denini08 denini08 force-pushed the v1.x.x branch 4 times, most recently from 54851f9 to 1ffd5af Compare August 23, 2025 12:17
@denini08
Copy link
Contributor Author

@llucax thanks for the feedback, can you please check now?

Copy link
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM. Just a couple of nitpick comments, but for me this is basically ready to be merged if tests pass.

@llucax llucax linked an issue Aug 25, 2025 that may be closed by this pull request
@llucax
Copy link
Contributor

llucax commented Aug 25, 2025

@denini08 we also require all commits to have verified signatures to be able to merge.

@denini08 denini08 force-pushed the v1.x.x branch 4 times, most recently from b2131f3 to e345e81 Compare August 25, 2025 13:08
@denini08
Copy link
Contributor Author

@llucax can you please check again?

@llucax
Copy link
Contributor

llucax commented Aug 25, 2025

Thanks again for taking the time for this @denini08! I was sort of live-watching as you fixed all the many issue with GPG signing and DCO, I'm sorry the process is so complicated!

All seems to be looking good now! Will merge when ready.

@github-project-automation github-project-automation bot moved this from To do to Review approved in Python SDK Roadmap Aug 25, 2025
@llucax llucax enabled auto-merge August 25, 2025 13:13
@llucax llucax changed the title fix missing lib to run tests on Windows Fix missing lib to run tests on Windows Aug 25, 2025
@llucax llucax changed the title Fix missing lib to run tests on Windows Remove unncessary use of tzdata in tests Aug 25, 2025
@llucax
Copy link
Contributor

llucax commented Aug 25, 2025

I took the liberty of updating the PR title to more accurately reflect what it does.

@llucax llucax added this pull request to the merge queue Aug 25, 2025
Merged via the queue into frequenz-floss:v1.x.x with commit c2e45b3 Aug 25, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from Review approved to Done in Python SDK Roadmap Aug 25, 2025
@denini08
Copy link
Contributor Author

Thanks again for taking the time for this @denini08! I was sort of live-watching as you fixed all the many issue with GPG signing and DCO, I'm sorry the process is so complicated!

All seems to be looking good now! Will merge when ready.

It's a difficult process, but it was good that I learned some new things

@llucax llucax modified the milestones: v1.0.0-rc2200, v1.0.0-rc2101 Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

Development

Successfully merging this pull request may close these issues.

Tests fail on Windows due to missing tzdata package

2 participants