Improve parametrization for unit conversion tests#84808
Conversation
937d2ed to
1b8cab9
Compare
1b8cab9 to
a481e59
Compare
| _CONVERTERS: list[type[BaseUnitConverter]] = [ | ||
| obj | ||
| for _, obj in inspect.getmembers(unit_conversion) | ||
| if inspect.isclass(obj) | ||
| and issubclass(obj, BaseUnitConverter) | ||
| and obj != BaseUnitConverter | ||
| ] |
There was a problem hiding this comment.
Another possibility is to make this list explicit, and then add a unit test to ensure that the explicit list contains all converters.
There was a problem hiding this comment.
Sometimes I wonder if we should use the Hypothesis library in HA for cases like these.
| return next( | ||
| filter( | ||
| lambda v: v != converter.NORMALIZED_UNIT, | ||
| sorted(converter.VALID_UNITS), |
There was a problem hiding this comment.
Filtering and then sorting, means you need to sort fewer items, than sorting and then filtering.
| INVALID_SYMBOL = "bob" | ||
| _CONVERTERS: dict[type[BaseUnitConverter], list[str]] = { | ||
| obj: sorted(obj.VALID_UNITS) | ||
| for _, obj in inspect.getmembers(unit_conversion) |
There was a problem hiding this comment.
It's nice that we don't need to update this container, but it's not very readable what the contents is besides the type.
There was a problem hiding this comment.
Yeah I think it might be better to add a test to ensure all converters are tested instead.
There was a problem hiding this comment.
@MartinHjelmare I've opened #86271 as an alternative.
I think it is better to keep the generated list just for comparing against the manual list and not for actually running the tests.
Proposed change
Improve parametrization for unit conversion tests
This will help ensure that new units do not creep into the unit enums or the converter valid units without the corresponding ratio.
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: