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

Fix for #3345 - precipitation probability not displayed when it is 0% #3346

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

vppencilsharpener
Copy link
Contributor

Fixes issue #3345.

I think I submitted this correctly, but don't do this often so let me know if anything needs to be corrected.

Treat zero values as valid data for Precipitation Probability.
@vppencilsharpener vppencilsharpener changed the title Develop Fix for #3345 - precipitation probability not displayed when it is 0% Jan 11, 2024
@khassel khassel requested a review from rejas January 11, 2024 20:29
@rejas
Copy link
Collaborator

rejas commented Jan 27, 2024

Sorry for not reviewing earlier... Might it be possible that you write a test for this?

@vppencilsharpener
Copy link
Contributor Author

Sorry for not reviewing earlier... Might it be possible that you write a test for this?

I am willing, but in all honesty I have never written an automated test so there is going to be a bit of a learning curve. If I could be directed to something existing I can base it off of that would be appreciated.

@rejas
Copy link
Collaborator

rejas commented Jan 28, 2024

Sorry for not reviewing earlier... Might it be possible that you write a test for this?

I am willing, but in all honesty I have never written an automated test so there is going to be a bit of a learning curve. If I could be directed to something existing I can base it off of that would be appreciated.

You can take a look at tests/e2e/modules/weather_hourly_spec.js
There are tests for checking the precipitation is displayed. Your test should check if the 0 is displayed if the pop in the mocked data tests/mocks/weather_hourly.json is 0

@rejas
Copy link
Collaborator

rejas commented Mar 21, 2024

Merging this so the next release has it. If you find the time @vppencilsharpener you can always add a test later ;-)

@rejas rejas merged commit d970214 into MagicMirrorOrg:develop Mar 21, 2024
5 checks passed
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.

2 participants