ha-snowtire
ha-snowtire copied to clipboard
Forecast fix for Home Assistant >= 2024.4.0
Breaking change
Proposed change
Fix support for new service request for forecast
Type of change
- [x] Dependency upgrade
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (which adds functionality to an this integration)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [x] Code quality improvements to existing code or addition of tests
Example entry for configuration.yaml
:
# Example configuration.yaml
Additional information
- This PR fixes or closes issue: fixes #149
Checklist
- [x] The code change is tested and works locally.
- [x] There is no commented out code in this PR.
- [x] The code has been formatted using Black (
black --fast custom_components
)
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated to README.md
I tried my best for tests to pass, both pre-commit
and CI tests but I had trouble with mypy
and pylint
local tests.
Also, testing the integration itself was challenging since implementing a mock weather integration is something I'm not familiar with. I've tested with various weather providers that implement the new get_forecasts
service request and it's working without issues.
@Limych any chance for a review? 😃
Please, fix errors in requirements and add unit tests for your code.
@Limych Thanks for taking a look at this PR. As I mentioned earlier, I can't fix all the tests needed for this PR.
I've fixed the lint testing but I can't fix "Test Package" because I don't know how to create a mock weather integration for testing.
For the moment, I included @pytest.mark.skip(reason="Can't mock a weather integration")
on the test_async_update
so I can make sure that the other test, which I know how they work, can run and pass.
I hope you can take this PR and make the modifications yourself so the main test can pass. If somebody else knows how to mock a service call to a weather integration, and can help me with it, I'd appreciate it. At the moment, this is as far as I can go.
For anybody else following this PR and needs a fix immediately, check the two following files in this PR (or my fork), and copy/paste them under your setup:
custom_components/snowtire/binary_sensor.py
custom_components/snowtire/manifest.json
All the other files changed in this PR are only for the tests, pre-commit, dev-container, etc; and aren't needed for the actual integration.
Ok, thanks for your work.
Unfortunately, I cannot accept PR without full-fledged unit tests, because then no one will do them and this will entail a deterioration in the quality of the code of the entire component. And right now, unfortunately, I don’t have the opportunity to work on this component yet.
@Limych after some googling and reading, I found out how to mock a service call. The unit test has been fixed to implement it, and the tests should be passing. Can you approve it so the workflow runs? If there are any additional errors after that, I could fix them.
@Limych please merge it until the end of October 😅
Edit: Also please approve the workflow
Thanks :heart: