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
Add missing conversion tests in unit conversion #86434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that diff isn't very friendly on this PR.
Most of the code change is just moving stuff around, merging the parametrisation of all the test_XXX_convert
into a single dictionary constant.
unit_ratio_item = _GET_UNIT_RATIO[converter] | ||
assert unit_ratio_item[0] != unit_ratio_item[1], "ratio units should be different" | ||
|
||
assert converter in _CONVERTED_VALUE, "converter is not present in _CONVERTED_VALUE" | ||
converted_value_items = _CONVERTED_VALUE[converter] | ||
for valid_unit in converter.VALID_UNITS: | ||
assert any( | ||
item | ||
for item in converted_value_items | ||
# item[1] is from_unit, item[3] is to_unit | ||
if valid_unit in {item[1], item[3]} | ||
), f"Unit `{valid_unit}` is not tested in _CONVERTED_VALUE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the second half of the code change: ensure that all units are in the _CONVERTED_VALUE
parametrization dictionary.
# Dict containing a conversion test for every know unit. | ||
_CONVERTED_VALUE: dict[ | ||
type[BaseUnitConverter], list[tuple[float, str | None, float, str | None]] | ||
] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first half of the PR: merged the parametrization lists into a single dictionary.
@pytest.mark.parametrize( | ||
"converter,value,from_unit,expected,to_unit", | ||
[ | ||
# Process all items in _CONVERTED_VALUE | ||
(converter, list_item[0], list_item[1], list_item[2], list_item[3]) | ||
for converter, item in _CONVERTED_VALUE.items() | ||
for list_item in item | ||
], | ||
) | ||
def test_unit_conversion( | ||
converter: type[BaseUnitConverter], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is where we use the new dictionary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks @epenet
../Frenck
Proposed change
As follow-up to #86340
ElectricPotentialConverter
,UnitlessRatioConverter
didn't have unit conversion tested.PressureConverter
was also missing a test forUnitOfPressure.BAR
.Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: