Skip to content

Mock calls to Open-Meteo API #281

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

listopadiya
Copy link

@listopadiya listopadiya commented May 12, 2025

Pull Request

Description

While executing unit tests in scope of github testing workflow, some requests to Open-Meteo API take a lot of time, and then fall with timeout (probably due to large timespans or authorization issues). It is considered to be not a good practice at all - to do network requests inside of unit tests. In scope of this fix, calls to Open-Meteo API are mocked with a fixture, which leads to smaller test execution time.

It was needed to do changes in quartz_solar_forecast/data.py and quartz_solar_forecast/forecasts/v2.py, so that they both use a wrapper from quartz_solar_forecast/weather/open_meteo.py, which made it possible to mock data requesting-related method. Also it allowed to avoid some of the code duplication.

Fixes #231

How Has This Been Tested?

Workflow tests job for unit tests succeeds in fork branch, test execution time is about 3 minutes now.
image

Coverage changed slightly (59%->58%), seems to be due to amount of codelines decreasing 🙂
image

Job for integration tests is failing, but I guess it wasn't working already.

  • Yes

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation - not sure if those needed
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@listopadiya
Copy link
Author

Not sure if this repo is still alive... But just in case if it is, and it wasn't clear - this PR is ready for review.

@peterdudfield
Copy link
Contributor

Hey @listopadiya thanks for this. Sorry i will review now

@peterdudfield
Copy link
Contributor

There's quite a few changes here. I wonder if you could simplify some of these.

Could the mock test be aded without the other changes?

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.

CI tests take 20 mins
2 participants