Skip to content

Commit c5364b4

Browse files
Add clarification about from_string tests limitations
`test_to_and_from_string()` is **extremely** fragile, even the most innocent change can break it, so we add some extra comments to make sure other people don't attempt to change it and avoid them some headaches. Signed-off-by: Leandro Lucarella <[email protected]> Co-authored-by: daniel-zullo-frequenz <[email protected]>
1 parent d7418b9 commit c5364b4

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

tests/timeseries/test_quantities.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ class Fz2(
5050

5151
_CtorType = Callable[[float], Quantity]
5252

53-
# Thi is the current number of subclasses. This probably will get outdated, but it will
53+
# This is the current number of subclasses. This probably will get outdated, but it will
5454
# provide at least some safety against something going really wrong and end up testing
55-
# an empty list. With this we should at least make sure we are not testint less classes
55+
# an empty list. With this we should at least make sure we are not testing less classes
5656
# than before. We don't get the actual number using len(_QUANTITY_SUBCLASSES) because it
5757
# would defeat the purpose of the test.
5858
_SANITFY_NUM_CLASSES = 7
@@ -65,7 +65,7 @@ class Fz2(
6565
)
6666
]
6767

68-
# A very basic sainty check that we are messing up the introspection
68+
# A very basic sanity check that are messing up the introspection
6969
assert len(_QUANTITY_SUBCLASSES) >= _SANITFY_NUM_CLASSES
7070

7171
_QUANTITY_BASE_UNIT_STRINGS = [
@@ -85,7 +85,7 @@ class Fz2(
8585
and m.__name__ != ("from_string"),
8686
)
8787
]
88-
# A very basic sainty check that we are messing up the introspection. There are actually
88+
# A very basic sanity check that are messing up the introspection. There are actually
8989
# many more constructors than classes, but this still works as a very basic check.
9090
assert len(_QUANTITY_CTORS) >= _SANITFY_NUM_CLASSES
9191

@@ -609,6 +609,9 @@ def test_invalid_multiplications() -> None:
609609
quantity *= 200.0 # type: ignore
610610

611611

612+
# We can't use _QUANTITY_TYPES here, because it will break the tests, as hypothesis
613+
# will generate more values, some of which are unsupported by the quantities. See the
614+
# test comment for more details.
612615
@pytest.mark.parametrize("quantity_type", [Power, Voltage, Current, Energy, Frequency])
613616
@pytest.mark.parametrize("exponent", [0, 3, 6, 9])
614617
@hypothesis.settings(
@@ -633,8 +636,14 @@ def test_to_and_from_string(
633636
generation and regenerate it with the more appropriate unit and precision.
634637
"""
635638
quantity = quantity_type.__new__(quantity_type)
636-
# pylint: disable=protected-access
637-
quantity._base_value = value * 10**exponent
639+
quantity._base_value = value * 10**exponent # pylint: disable=protected-access
640+
# The above should be replaced with:
641+
# quantity = quantity_type._new( # pylint: disable=protected-access
642+
# value, exponent=exponent
643+
# )
644+
# But we can't do that now, because, you guessed it, it will also break the tests
645+
# (_new() will use 10.0**exponent instead of 10**exponent, which seems to have some
646+
# effect on the tests.
638647
quantity_str = f"{quantity:.{exponent}}"
639648
from_string = quantity_type.from_string(quantity_str)
640649
try:

0 commit comments

Comments
 (0)