diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index b7b191eb..5564d68e 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -52,6 +52,16 @@ A well-formed `DeliveryArea` has a non-empty `code` and a specified (non-`UNSPECIFIED`) `code_type`. Constructing one with invalid data currently emits a `DeprecationWarning`; a future release will replace the warning with a hard `ValueError`. To opt into the upcoming behavior right now, pass `_raise_on_invalid=True` to the constructor. Prefer `delivery_area_from_proto2` to load delivery areas from the wire — malformed messages become `InvalidDeliveryArea` instances instead. +* `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto` is now deprecated; use `bounds_from_proto2` instead. + + The new converter returns `Bounds | InvalidBounds` and surfaces malformed wire data at the type level rather than raising a `ValueError` when `lower > upper`. The old converter continues to work but emits a `DeprecationWarning`. + +* `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto_with_issues` is now deprecated with no direct replacement. + + Validity is now encoded in the return type of `bounds_from_proto2` (`Bounds | InvalidBounds`), so callers should inspect the returned type instead of collecting issue strings via a side channel. The old converter continues to work but emits a `DeprecationWarning`. + +* `frequenz.client.common.metrics.Bounds.__str__` now renders as `[lower,upper]` (no space after the comma) to match the compact format used by `Lifetime` and to compose cleanly with the `` marker on `InvalidBounds`. + ## New Features * Added 4 new electrical component classes for categories that previously collapsed into `UnrecognizedElectricalComponent`: @@ -81,6 +91,7 @@ * `frequenz.client.common.grid.DeliveryArea.get_code_type()` * `frequenz.client.common.metrics.MetricConnection.get_category()` * `frequenz.client.common.metrics.MetricSample.get_metric()` + * `frequenz.client.common.microgrid.electrical_components.ElectricalComponent.get_metric_config_bounds()` * Added new delivery-area class hierarchy: @@ -92,6 +103,8 @@ * Added a new `frequenz.client.common.microgrid.Lifetime` type together with the `frequenz.client.common.microgrid.proto.v1alpha8.lifetime_from_proto` conversion function. +* Added `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto2` returning `Bounds | InvalidBounds`. This is the replacement for the now-deprecated `bounds_from_proto`. + * Added a new `frequenz.client.common.types.Location` type together with the `frequenz.client.common.types.proto.v1alpha8.location_from_proto` conversion function. * Added a new `frequenz.client.common.microgrid.Microgrid` type, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. @@ -100,6 +113,8 @@ The class of a component is its identity; components don't carry category or type attributes. The only exceptions are the error-recovery classes `UnrecognizedElectricalComponent` and `MismatchedCategoryElectricalComponent` (with a raw protobuf `category` value) and `UnrecognizedBattery`, `UnrecognizedInverter` and `UnrecognizedEvCharger` (with a raw protobuf `type` value), which preserve the raw protobuf values received from the protocol version used to load them. + `ElectricalComponent.metric_config_bounds` is typed `Mapping[Metric | int, Bounds | InvalidBounds]`: malformed wire entries are preserved as `InvalidBounds` instead of being silently dropped, and entries that named a metric but carried no (or an empty) `config_bounds` submessage load as an unbounded `Bounds()` (a `Bounds` with neither bound set imposes no limit in either direction). Use `get_metric_config_bounds()` to resolve an entry to a valid `Bounds` or a clear `InvalidBoundsError`; it mimics `dict.get()`, returning an unbounded `Bounds()` (or a caller-supplied `default`) for absent metrics. + * Added a new `frequenz.client.common.microgrid.Microgrid` type with a raising `is_active()` method, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. ## Bug Fixes diff --git a/src/frequenz/client/common/metrics/__init__.py b/src/frequenz/client/common/metrics/__init__.py index 45a2bbba..987f203c 100644 --- a/src/frequenz/client/common/metrics/__init__.py +++ b/src/frequenz/client/common/metrics/__init__.py @@ -3,7 +3,7 @@ """Metrics definitions.""" -from ._bounds import Bounds +from ._bounds import BaseBounds, Bounds, InvalidBounds, InvalidBoundsError from ._metric import Metric from ._sample import ( AggregatedMetricValue, @@ -16,7 +16,10 @@ __all__ = [ "AggregatedMetricValue", "AggregationMethod", + "BaseBounds", "Bounds", + "InvalidBounds", + "InvalidBoundsError", "Metric", "MetricConnection", "MetricConnectionCategory", diff --git a/src/frequenz/client/common/metrics/_bounds.py b/src/frequenz/client/common/metrics/_bounds.py index de59bde5..30896b9a 100644 --- a/src/frequenz/client/common/metrics/_bounds.py +++ b/src/frequenz/client/common/metrics/_bounds.py @@ -5,29 +5,54 @@ """Definitions for bounds.""" import dataclasses +from typing import Any, Self +from .._exception import InvalidAttributeError -@dataclasses.dataclass(frozen=True, kw_only=True) -class Bounds: - """A set of lower and upper bounds for any metric. - The lower bound must be less than or equal to the upper bound. +@dataclasses.dataclass(frozen=True, kw_only=True) +class BaseBounds: + """A base class for well-formed and malformed metric bounds. - The units of the bounds are always the same as the related metric. + This class cannot be instantiated directly. Use [`Bounds`][..Bounds] for a + valid pair of bounds or [`InvalidBounds`][..InvalidBounds] to preserve + malformed wire data. """ - lower: float | None = None + lower: float | int | None = None """The lower bound. If `None`, there is no lower bound. """ - upper: float | None = None + upper: float | int | None = None """The upper bound. If `None`, there is no upper bound. """ + # pylint: disable-next=unused-argument + def __new__(cls, *args: Any, **kwargs: Any) -> Self: + """Prevent instantiation of this class.""" + if cls is BaseBounds: + raise TypeError(f"Cannot instantiate {cls.__name__} directly") + return super().__new__(cls) + + +@dataclasses.dataclass(frozen=True, kw_only=True) +class Bounds(BaseBounds): + """A set of lower and upper bounds for any metric. + + The lower bound must be less than or equal to the upper bound. + + The units of the bounds are always the same as the related metric. + + Note: + Raises a `ValueError` if [`lower`][.lower] is greater than + [`upper`][.upper]. Use [`InvalidBounds`][..InvalidBounds] to + represent malformed bounds data received from the wire. + """ + def __post_init__(self) -> None: """Validate these bounds.""" if self.lower is None: @@ -42,4 +67,59 @@ def __post_init__(self) -> None: def __str__(self) -> str: """Return a string representation of these bounds.""" - return f"[{self.lower}, {self.upper}]" + return f"[{self.lower},{self.upper}]" + + +@dataclasses.dataclass(frozen=True, kw_only=True) +class InvalidBounds(BaseBounds): + """Metric bounds with malformed data received from the wire. + + This class preserves bounds data that fails the invariants required for + a well-formed [`Bounds`][..Bounds], allowing callers to inspect the raw + values without accidentally using them for range checks. Use a semantic + accessor, such as `ElectricalComponent.get_metric_config_bounds()`, to + receive a clear error on invalid data. + """ + + def __str__(self) -> str: + """Return a compact string representation of these invalid bounds.""" + return f"" + + +class InvalidBoundsError(InvalidAttributeError): + """Raised when a semantic accessor sees invalid metric bounds. + + The offending [`InvalidBounds`][..InvalidBounds] is available as the + [`bounds`][.bounds] attribute so callers can inspect the raw wire data. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + bounds: InvalidBounds, + message: str | None = None, + ) -> None: + """Initialize this error. + + Args: + instance: The instance that was being accessed when this error was raised. + attr_name: The name of the attribute that was being accessed. + bounds: The invalid bounds instance. + message: A custom error message. If `None`, a default message mentioning + the invalid bounds is used. + """ + self.bounds: InvalidBounds = bounds + """The invalid bounds that caused this error.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid bounds {bounds!r} for attribute {attr_name!r} in {instance}" + ), + ) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py index efc7c6f0..757a0542 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py @@ -3,7 +3,11 @@ """Conversion of metrics enums from/to protobuf v1alpha8.""" -from ._bounds import bounds_from_proto, bounds_from_proto_with_issues +from ._bounds import ( + bounds_from_proto, + bounds_from_proto2, + bounds_from_proto_with_issues, +) from ._metric import metric_from_proto, metric_to_proto from ._metric_connection_category import ( metric_connection_category_from_proto, @@ -18,6 +22,7 @@ __all__ = [ "aggregated_metric_sample_from_proto", "bounds_from_proto", + "bounds_from_proto2", "bounds_from_proto_with_issues", "metric_connection_category_from_proto", "metric_connection_category_to_proto", diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index 8e9fba4f..bce06af7 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -4,13 +4,24 @@ """Loading of Bounds objects from protobuf messages.""" from frequenz.api.common.v1alpha8.metrics import bounds_pb2 +from typing_extensions import deprecated -from ..._bounds import Bounds +from ..._bounds import Bounds, InvalidBounds +@deprecated( + "`bounds_from_proto` is deprecated; use " + "`bounds_from_proto2` (returns `Bounds | InvalidBounds`) instead." +) def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 """Create a [`Bounds`][....Bounds] object from a protobuf message. + Warning: Deprecated + Use [`bounds_from_proto2`][..bounds_from_proto2] instead. The new + converter distinguishes well-formed from malformed data at the + type level (`Bounds | InvalidBounds`) rather than raising a + `ValueError` when the invariant fires. + Args: message: The protobuf message to convert. @@ -26,6 +37,34 @@ def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 ) +def bounds_from_proto2( + message: bounds_pb2.Bounds, +) -> Bounds | InvalidBounds: + """Create bounds from a protobuf message, preserving malformed data. + + Args: + message: The protobuf message to convert. + + Returns: + A [`Bounds`][....Bounds] when the values form a valid range, or an + [`InvalidBounds`][....InvalidBounds] preserving values that + violate `lower <= upper`. A present but empty protobuf message + becomes an unbounded `Bounds()`. + """ + lower = message.lower if message.HasField("lower") else None + upper = message.upper if message.HasField("upper") else None + try: + return Bounds(lower=lower, upper=upper) + except ValueError: + pass + return InvalidBounds(lower=lower, upper=upper) + + +@deprecated( + "`bounds_from_proto_with_issues` is deprecated; use " + "`bounds_from_proto2` (returns `Bounds | InvalidBounds`) and inspect " + "the returned type instead." +) def bounds_from_proto_with_issues( message: bounds_pb2.Bounds, *, @@ -34,6 +73,13 @@ def bounds_from_proto_with_issues( ) -> Bounds | None: # noqa: DOC502 """Create a [`Bounds`][....Bounds] object from a protobuf message, collecting issues. + Warning: Deprecated + Use [`bounds_from_proto2`][..bounds_from_proto2] instead and + inspect the returned type. The new converter distinguishes + well-formed from malformed data at the type level + (`Bounds | InvalidBounds`) rather than routing invalid data + through a side-channel string list. + Args: message: The protobuf message to convert. major_issues: A list to append major issues to. @@ -43,7 +89,10 @@ def bounds_from_proto_with_issues( The corresponding [`Bounds`][....Bounds] object. """ try: - return bounds_from_proto(message) + return Bounds( + lower=message.lower if message.HasField("lower") else None, + upper=message.upper if message.HasField("upper") else None, + ) except ValueError as exc: major_issues.append(str(exc)) return None diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py index 6e804abe..5eaef157 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py @@ -4,11 +4,12 @@ """Loading of MetricSample and AggregatedMetricValue objects from protobuf messages.""" from collections.abc import Sequence +from typing import assert_never from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 from ....proto import datetime_from_proto -from ..._bounds import Bounds +from ..._bounds import Bounds, InvalidBounds from ..._metric import Metric from ..._sample import ( AggregatedMetricValue, @@ -16,7 +17,7 @@ MetricConnectionCategory, MetricSample, ) -from ._bounds import bounds_from_proto +from ._bounds import bounds_from_proto2 from ._metric import metric_from_proto from ._metric_connection_category import metric_connection_category_from_proto @@ -144,14 +145,16 @@ def _metric_bounds_from_proto( """ bounds: list[Bounds] = [] for pb_bound in messages: - try: - bound = bounds_from_proto(pb_bound) - except ValueError as exc: - metric_name = metric if isinstance(metric, int) else metric.name - major_issues.append( - f"bounds for {metric_name} is invalid ({exc}), ignoring these bounds" - ) - continue - bounds.append(bound) + match bounds_from_proto2(pb_bound): + case Bounds() as bound: + bounds.append(bound) + case InvalidBounds() as bound: + metric_name = metric if isinstance(metric, int) else metric.name + major_issues.append( + f"bounds for {metric_name} is invalid ({bound}), " + "ignoring these bounds" + ) + case unknown: + assert_never(unknown) return bounds diff --git a/src/frequenz/client/common/microgrid/electrical_components/__init__.py b/src/frequenz/client/common/microgrid/electrical_components/__init__.py index 380d67d0..5f4484bc 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/__init__.py +++ b/src/frequenz/client/common/microgrid/electrical_components/__init__.py @@ -18,7 +18,7 @@ from ._converter import Converter from ._crypto_miner import CryptoMiner from ._diagnostic_code import ElectricalComponentDiagnosticCode -from ._electrical_component import ElectricalComponent +from ._electrical_component import DefaultT, ElectricalComponent from ._electrical_component_connection import ( BaseElectricalComponentConnection, ElectricalComponentConnection, @@ -85,6 +85,7 @@ "Converter", "CryptoMiner", "DcEvCharger", + "DefaultT", "ElectricalComponent", "ElectricalComponentCategory", "ElectricalComponentConnection", diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index ee74b0a0..2b225cbe 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -6,14 +6,17 @@ import dataclasses from collections.abc import Mapping from datetime import datetime, timezone -from typing import Any, Self, assert_never +from typing import Any, Self, TypeVar, assert_never, overload from ..._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError -from ...metrics import Bounds, Metric +from ...metrics import Bounds, InvalidBounds, InvalidBoundsError, Metric from .. import MicrogridId from .._lifetime import InvalidLifetime, InvalidLifetimeError, Lifetime from ._ids import ElectricalComponentId +DefaultT = TypeVar("DefaultT") +"""A type variable for the default value of dict-like getters.""" + @dataclasses.dataclass(frozen=True, kw_only=True) class ElectricalComponent: # pylint: disable=too-many-instance-attributes @@ -72,22 +75,32 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes ) """Internal guard allowing construction only via the `*_from_proto` converters.""" - metric_config_bounds: Mapping[Metric | int, Bounds] = dataclasses.field( - default_factory=dict, - # dict is not hashable, so we don't use this field to calculate the hash. This - # shouldn't be a problem since it is very unlikely that two components with all - # other attributes being equal would have different category specific metadata, - # so hash collisions should be still very unlikely. - hash=False, + metric_config_bounds: Mapping[Metric | int, Bounds | InvalidBounds] = ( + dataclasses.field( + default_factory=dict, + # dict is not hashable, so we don't use this field to calculate the hash. + # This shouldn't be a problem since it is very unlikely that two components + # with all other attributes being equal would have different category + # specific metadata, so hash collisions should be still very unlikely. + hash=False, + ) ) """The metric configuration bounds for this electrical component, keyed by metric. These bounds may be derived from the component configuration, manufacturer limits, or limits of other devices. + Malformed bounds received from the wire are preserved as + [`InvalidBounds`][.....metrics.InvalidBounds] instances so callers can + inspect the raw values without accidentally using them for range checks. + If an unspecified metric is received, it is stored as the plain `int` key `0` when loading from protobuf. Metrics unknown to this client version may also appear as plain `int` keys for forward-compatibility. + + Tip: + Prefer [`get_metric_config_bounds()`][..get_metric_config_bounds] + when a valid [`Bounds`][.....metrics.Bounds] is required. """ category_specific_metadata: Mapping[str, Any] = dataclasses.field( @@ -192,6 +205,68 @@ def accepts_control(self) -> bool: case unknown: assert_never(unknown) + @overload + def get_metric_config_bounds(self, metric: Metric) -> Bounds: ... + + @overload + def get_metric_config_bounds( + self, metric: Metric, *, default: DefaultT + ) -> Bounds | DefaultT: ... + + def get_metric_config_bounds( + self, metric: Metric, *, default: object = Bounds() + ) -> object: + """Return the configured bounds for a metric as a valid `Bounds`. + + An absent entry returns an unbounded metric, so when no bounds are + configured for `metric` this returns an unbounded + [`Bounds`][frequenz.client.common.metrics.Bounds] by default. Pass + `default` to return a different value for absent entries instead, + mimicking [`dict.get()`][dict.get]. + + Example: + To check if a `metric` has **valid** configured bounds, you can use: + + ```py + component: ElectricalComponent + metric: Metric + if component.get_metric_config_bounds(metric, default=None) is not None: + print(f"{metric} has valid configured bounds") + ``` + + This is similar to accessing + [`metric_config_bounds`][...ElectricalComponent.metric_config_bounds] + directly, but avoid the special handling of invalid bounds. + + Args: + metric: The metric whose bounds to retrieve. + default: The value to return when no bounds are configured for + `metric`. + + Returns: + The valid [`Bounds`][.....metrics.Bounds] configured for `metric`, + or `default` when there is no entry for `metric`. + + Raises: + InvalidBoundsError: If the bounds configured for `metric` are + malformed. The offending instance is available on the + exception's `bounds` attribute. + """ + match self.metric_config_bounds.get(metric): + case None: + return default + case InvalidBounds() as invalid: + raise InvalidBoundsError( + self, + "metric_config_bounds", + invalid, + f"invalid bounds {invalid!r} for metric {metric} in {self}", + ) + case Bounds() as valid: + return valid + case unknown: + assert_never(unknown) + def get_operational_lifetime(self) -> Lifetime: """Return the operational lifetime as a valid `Lifetime`. diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 4a169f1a..8bc40b6e 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -13,8 +13,8 @@ ) from google.protobuf.json_format import MessageToDict -from .....metrics import Bounds, Metric -from .....metrics.proto.v1alpha8 import bounds_from_proto +from .....metrics import Bounds, InvalidBounds, Metric +from .....metrics.proto.v1alpha8 import bounds_from_proto2 from .....proto import enum_from_proto from ...._ids import MicrogridId from ...._lifetime import InvalidLifetime, Lifetime @@ -891,8 +891,14 @@ class _ElectricalComponentBaseData(NamedTuple): lifetime: Lifetime | InvalidLifetime """The operational lifetime of the electrical component.""" - metric_config_bounds: dict[Metric | int, Bounds] - """The metric configuration bounds extracted from the protobuf message.""" + metric_config_bounds: dict[Metric | int, Bounds | InvalidBounds] + """The metric configuration bounds extracted from the protobuf message. + + Malformed entries are preserved as + [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds]; entries + whose `config_bounds` field was not set load as an unbounded + [`Bounds`][frequenz.client.common.metrics.Bounds]. + """ category_specific_info: dict[str, Any] """The category-specific metadata extracted from the protobuf message.""" @@ -912,7 +918,7 @@ def _electrical_component_base_from_proto_with_issues( message: electrical_components_pb2.ElectricalComponent, *, major_issues: list[str], - minor_issues: list[str], + minor_issues: list[str], # pylint: disable=unused-argument ) -> _ElectricalComponentBaseData: """Extract base data from a protobuf message and collect issues. @@ -936,9 +942,7 @@ def _electrical_component_base_from_proto_with_issues( lifetime = _get_operational_lifetime_from_proto(message) metric_config_bounds = _metric_config_bounds_from_proto( - message.metric_config_bounds, - major_issues=major_issues, - minor_issues=minor_issues, + message.metric_config_bounds ) category = enum_from_proto(message.category, ElectricalComponentCategory) @@ -1207,59 +1211,40 @@ def electrical_component_from_proto_with_issues( def _metric_config_bounds_from_proto( message: Sequence[electrical_components_pb2.MetricConfigBounds], - *, - major_issues: list[str], - minor_issues: list[str], # pylint: disable=unused-argument -) -> dict[Metric | int, Bounds]: - """Convert a `MetricConfigBounds` message to a dictionary mapping `Metric` to `Bounds`. +) -> dict[Metric | int, Bounds | InvalidBounds]: + """Convert a `MetricConfigBounds` message to a dictionary mapping `Metric` to bounds. The keys of the result map are [`Metric`][frequenz.client.common.metrics.Metric] enum members (or `int` for - unrecognized values) and the values are - [`Bounds`][frequenz.client.common.metrics.Bounds] objects. + unrecognized values). Values are + [`Bounds`][frequenz.client.common.metrics.Bounds] for well-formed entries and + [`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] for entries + that carried bound values violating `lower <= upper`. + + An entry with no configured limits — its `config_bounds` submessage absent, + or present but empty — loads as an unbounded + [`Bounds`][frequenz.client.common.metrics.Bounds]: a `Bounds` with neither + `lower` nor `upper` set imposes no limit in either direction. Absence and a + present-but-empty submessage are intentionally treated the same. + + Duplicated metrics on the wire follow proto3 map semantics: the last entry + wins silently. Args: message: The `MetricConfigBounds` message. - major_issues: A list to append major issues to. - minor_issues: A list to append minor issues to. Returns: The resulting dictionary mapping metrics to their bounds. """ - bounds: dict[Metric | int, Bounds] = {} + bounds: dict[Metric | int, Bounds | InvalidBounds] = {} for metric_bound in message: with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=DeprecationWarning) metric = enum_from_proto(metric_bound.metric, Metric) - match metric: - case Metric.UNSPECIFIED: - metric = metric.value - case int(): - minor_issues.append( - f"metric_config_bounds has an unrecognized metric {metric}" - ) - - if not metric_bound.HasField("config_bounds"): - major_issues.append( - f"metric_config_bounds for {metric} is present but missing " - "`config_bounds`, considering it unbounded", - ) - continue + if metric is Metric.UNSPECIFIED: + metric = metric.value - try: - bound = bounds_from_proto(metric_bound.config_bounds) - except ValueError as exc: - major_issues.append( - f"metric_config_bounds for {metric} is invalid ({exc}), considering " - "it as missing (i.e. unbouded)", - ) - continue - if metric in bounds: - major_issues.append( - f"metric_config_bounds for {metric} is duplicated in the message" - f"using the last one ({bound})", - ) - bounds[metric] = bound + bounds[metric] = bounds_from_proto2(metric_bound.config_bounds) return bounds diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index ff220472..5a7ae505 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -3,8 +3,6 @@ """Loading of Microgrid objects from protobuf messages.""" -import logging - from frequenz.api.common.v1alpha8.microgrid import microgrid_pb2 from ....grid import DeliveryArea, InvalidDeliveryArea @@ -15,9 +13,6 @@ from ..._ids import EnterpriseId, MicrogridId from ..._microgrid import Microgrid -_logger = logging.getLogger(__name__) - - _ACTIVE_BY_STATUS: dict[int, bool] = { microgrid_pb2.MICROGRID_STATUS_ACTIVE: True, microgrid_pb2.MICROGRID_STATUS_INACTIVE: False, @@ -49,32 +44,13 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: Returns: The corresponding [`Microgrid`][....Microgrid] object. """ - major_issues: list[str] = [] - delivery_area: DeliveryArea | InvalidDeliveryArea | None = None if message.HasField("delivery_area"): delivery_area = delivery_area_from_proto2(message.delivery_area) - else: - major_issues.append("delivery_area is missing") location: Location | None = None if message.HasField("location"): location = location_from_proto(message.location) - else: - major_issues.append("location is missing") - - active = _microgrid_status_to_active(message.status) - if message.status == microgrid_pb2.MICROGRID_STATUS_UNSPECIFIED: - major_issues.append("status is unspecified") - elif not isinstance(active, bool): - major_issues.append("status is unrecognized") - - if major_issues: - _logger.warning( - "Found issues in microgrid: %s | Protobuf message:\n%s", - ", ".join(major_issues), - message, - ) # The wrapper uses create_time, but the protobuf field remains create_timestamp. return Microgrid( @@ -84,6 +60,6 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: delivery_area=delivery_area, location=location, create_time=datetime_from_proto(message.create_timestamp), - _active=active, + _active=_microgrid_status_to_active(message.status), _allow_construction=True, ) diff --git a/tests/metrics/_bounds/__init__.py b/tests/metrics/_bounds/__init__.py new file mode 100644 index 00000000..221b7955 --- /dev/null +++ b/tests/metrics/_bounds/__init__.py @@ -0,0 +1,4 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for bounds types.""" diff --git a/tests/metrics/_bounds/test_base_bounds.py b/tests/metrics/_bounds/test_base_bounds.py new file mode 100644 index 00000000..a50ed989 --- /dev/null +++ b/tests/metrics/_bounds/test_base_bounds.py @@ -0,0 +1,14 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `BaseBounds`.""" + +import pytest + +from frequenz.client.common.metrics import BaseBounds + + +def test_cannot_be_instantiated_directly() -> None: + """`BaseBounds` refuses direct instantiation.""" + with pytest.raises(TypeError, match="Cannot instantiate BaseBounds directly"): + BaseBounds() diff --git a/tests/metrics/test_bounds.py b/tests/metrics/_bounds/test_bounds.py similarity index 83% rename from tests/metrics/test_bounds.py rename to tests/metrics/_bounds/test_bounds.py index f33b22de..ee3d5dd7 100644 --- a/tests/metrics/test_bounds.py +++ b/tests/metrics/_bounds/test_bounds.py @@ -1,13 +1,18 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for the Bounds class.""" +"""Tests for `Bounds`.""" import re import pytest -from frequenz.client.common.metrics import Bounds +from frequenz.client.common.metrics import BaseBounds, Bounds + + +def test_is_base_bounds_subclass() -> None: + """`Bounds` is a subclass of `BaseBounds`.""" + assert issubclass(Bounds, BaseBounds) @pytest.mark.parametrize( @@ -24,7 +29,7 @@ (0.0, 0.0), ], ) -def test_creation(lower: float, upper: float) -> None: +def test_creation(lower: float | int | None, upper: float | int | None) -> None: """Test creation of Bounds with valid values.""" bounds = Bounds(lower=lower, upper=upper) assert bounds.lower == lower @@ -45,7 +50,7 @@ def test_invalid_values() -> None: def test_str_representation() -> None: """Test string representation of Bounds.""" bounds = Bounds(lower=-10.0, upper=10.0) - assert str(bounds) == "[-10.0, 10.0]" + assert str(bounds) == "[-10.0,10.0]" def test_equality() -> None: diff --git a/tests/metrics/_bounds/test_invalid_bounds.py b/tests/metrics/_bounds/test_invalid_bounds.py new file mode 100644 index 00000000..50506436 --- /dev/null +++ b/tests/metrics/_bounds/test_invalid_bounds.py @@ -0,0 +1,55 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidBounds`.""" + +from frequenz.client.common.metrics import BaseBounds, Bounds, InvalidBounds + + +def test_is_base_bounds_subclass() -> None: + """`InvalidBounds` is a subclass of `BaseBounds`.""" + assert issubclass(InvalidBounds, BaseBounds) + + +def test_is_not_bounds_subclass() -> None: + """`InvalidBounds` is a sibling of `Bounds`, not a subclass.""" + assert not issubclass(InvalidBounds, Bounds) + + +def test_accepts_invalid_range() -> None: + """`InvalidBounds` preserves an upper bound below its lower bound.""" + bounds = InvalidBounds(lower=10.0, upper=-10.0) + assert bounds.lower == 10.0 + assert bounds.upper == -10.0 + + +def test_accepts_valid_looking_values() -> None: + """`InvalidBounds` enforces no invariants and accepts any values.""" + bounds = InvalidBounds(lower=-10.0, upper=10.0) + assert bounds.lower == -10.0 + assert bounds.upper == 10.0 + + +def test_str_representation() -> None: + """`InvalidBounds.__str__` wraps the compact form in ``.""" + assert str(InvalidBounds(lower=10.0, upper=-10.0)) == "" + assert str(InvalidBounds()) == "" + + +def test_equality() -> None: + """Test equality comparison of `InvalidBounds` objects.""" + bounds1 = InvalidBounds(lower=10.0, upper=-10.0) + bounds2 = InvalidBounds(lower=10.0, upper=-10.0) + bounds3 = InvalidBounds(lower=5.0, upper=-5.0) + + assert bounds1 == bounds2 + assert bounds1 != bounds3 + + +def test_hash() -> None: + """Test that `InvalidBounds` objects can be used in sets and as dict keys.""" + bounds1 = InvalidBounds(lower=10.0, upper=-10.0) + bounds2 = InvalidBounds(lower=10.0, upper=-10.0) + bounds3 = InvalidBounds(lower=5.0, upper=-5.0) + + assert len({bounds1, bounds2, bounds3}) == 2 diff --git a/tests/metrics/_bounds/test_invalid_bounds_error.py b/tests/metrics/_bounds/test_invalid_bounds_error.py new file mode 100644 index 00000000..cd12e742 --- /dev/null +++ b/tests/metrics/_bounds/test_invalid_bounds_error.py @@ -0,0 +1,43 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidBoundsError`.""" + +from frequenz.client.common import InvalidAttributeError +from frequenz.client.common.metrics import InvalidBounds, InvalidBoundsError + + +def test_default_message() -> None: + """`InvalidBoundsError` builds a default message from the invalid bounds.""" + invalid = InvalidBounds(lower=10.0, upper=-10.0) + error = InvalidBoundsError("some-instance", "config_bounds", invalid) + + assert error.bounds is invalid + assert ( + str(error) == f"invalid bounds {invalid!r} for attribute 'config_bounds' " + "in some-instance" + ) + + +def test_custom_message() -> None: + """`InvalidBoundsError` accepts a custom message.""" + invalid = InvalidBounds(lower=10.0, upper=-10.0) + error = InvalidBoundsError( + "some-instance", + "config_bounds", + invalid, + message="bad bounds from server", + ) + + assert error.bounds is invalid + assert str(error) == "bad bounds from server" + + +def test_is_invalid_attribute_error() -> None: + """`InvalidBoundsError` is an `InvalidAttributeError`.""" + assert issubclass(InvalidBoundsError, InvalidAttributeError) + + +def test_is_value_error() -> None: + """`InvalidBoundsError` is a `ValueError`.""" + assert issubclass(InvalidBoundsError, ValueError) diff --git a/tests/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index 0444ccde..b839b722 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -8,8 +8,10 @@ import pytest from frequenz.api.common.v1alpha8.metrics import bounds_pb2 +from frequenz.client.common.metrics import Bounds, InvalidBounds from frequenz.client.common.metrics.proto.v1alpha8 import ( bounds_from_proto, + bounds_from_proto2, bounds_from_proto_with_issues, ) @@ -27,10 +29,10 @@ class ProtoConversionTestCase: has_upper: bool """Whether to include upper bound in the protobuf message.""" - lower: float | None + lower: float | int | None """The lower bound value to set.""" - upper: float | None + upper: float | int | None """The upper bound value to set.""" @@ -76,12 +78,20 @@ def test_from_proto(case: ProtoConversionTestCase) -> None: if case.has_upper and case.upper is not None: proto.upper = case.upper - bounds = bounds_from_proto(proto) + with pytest.deprecated_call(match="bounds_from_proto2"): + bounds = bounds_from_proto(proto) assert bounds.lower == case.lower assert bounds.upper == case.upper +def test_from_proto_emits_deprecation_warning() -> None: + """`bounds_from_proto` itself is deprecated and warns on call.""" + proto = bounds_pb2.Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(match="bounds_from_proto2"): + bounds_from_proto(proto) + + def test_from_proto_with_issues_valid() -> None: """Test bounds_from_proto_with_issues with valid bounds.""" proto = bounds_pb2.Bounds() @@ -91,9 +101,10 @@ def test_from_proto_with_issues_valid() -> None: major_issues: list[str] = [] minor_issues: list[str] = [] - bounds = bounds_from_proto_with_issues( - proto, major_issues=major_issues, minor_issues=minor_issues - ) + with pytest.deprecated_call(match="bounds_from_proto2"): + bounds = bounds_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) assert bounds is not None assert bounds.lower == -10.0 @@ -111,9 +122,10 @@ def test_from_proto_with_issues_invalid() -> None: major_issues: list[str] = [] minor_issues: list[str] = [] - bounds = bounds_from_proto_with_issues( - proto, major_issues=major_issues, minor_issues=minor_issues - ) + with pytest.deprecated_call(match="bounds_from_proto2"): + bounds = bounds_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) assert bounds is None assert len(major_issues) == 1 @@ -122,3 +134,87 @@ def test_from_proto_with_issues_invalid() -> None: in major_issues[0] ) assert not minor_issues + + +def test_from_proto_with_issues_emits_deprecation_warning() -> None: + """`bounds_from_proto_with_issues` itself is deprecated and warns on call.""" + proto = bounds_pb2.Bounds(lower=-10.0, upper=10.0) + major_issues: list[str] = [] + minor_issues: list[str] = [] + with pytest.deprecated_call(match="bounds_from_proto2"): + bounds_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) + + +@pytest.mark.parametrize( + "case", + [ + ProtoConversionTestCase( + name="full", + has_lower=True, + has_upper=True, + lower=-10.0, + upper=10.0, + ), + ProtoConversionTestCase( + name="no_upper_bound", + has_lower=True, + has_upper=False, + lower=-10.0, + upper=None, + ), + ProtoConversionTestCase( + name="no_lower_bound", + has_lower=False, + has_upper=True, + lower=None, + upper=10.0, + ), + ProtoConversionTestCase( + name="no_both_bounds", + has_lower=False, + has_upper=False, + lower=None, + upper=None, + ), + ], + ids=lambda case: case.name, +) +def test_from_proto2_valid(case: ProtoConversionTestCase) -> None: + """`bounds_from_proto2` returns a `Bounds` for well-formed messages.""" + proto = bounds_pb2.Bounds() + if case.has_lower and case.lower is not None: + proto.lower = case.lower + if case.has_upper and case.upper is not None: + proto.upper = case.upper + + bounds = bounds_from_proto2(proto) + + assert isinstance(bounds, Bounds) + assert not isinstance(bounds, InvalidBounds) + assert bounds.lower == case.lower + assert bounds.upper == case.upper + + +def test_from_proto2_invalid() -> None: + """`bounds_from_proto2` returns `InvalidBounds` when `lower > upper`.""" + proto = bounds_pb2.Bounds(lower=10.0, upper=-10.0) + + bounds = bounds_from_proto2(proto) + + assert isinstance(bounds, InvalidBounds) + assert not isinstance(bounds, Bounds) + assert bounds.lower == 10.0 + assert bounds.upper == -10.0 + + +def test_from_proto2_empty_message_is_unbounded_bounds() -> None: + """A present but empty protobuf message becomes an unbounded `Bounds()`.""" + bounds = bounds_from_proto2(bounds_pb2.Bounds()) + + assert isinstance(bounds, Bounds) + assert not isinstance(bounds, InvalidBounds) + assert bounds == Bounds() + assert bounds.lower is None + assert bounds.upper is None diff --git a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py index 54b82520..b17a1aa2 100644 --- a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py +++ b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py @@ -156,8 +156,8 @@ class _TestCase: ), expected_major_issues=[ ( - "bounds for AC_POWER_ACTIVE is invalid (Lower bound (10.0) must be " - "less than or equal to upper bound (-10.0)), ignoring these bounds" + "bounds for AC_POWER_ACTIVE is invalid (), " + "ignoring these bounds" ) ], ), diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 2f9a6a3c..e94a1a1b 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -12,7 +12,7 @@ ) from google.protobuf.timestamp_pb2 import Timestamp -from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common.metrics import Bounds, InvalidBounds, Metric from frequenz.client.common.microgrid import InvalidLifetime, Lifetime from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentCategory, @@ -201,7 +201,7 @@ def test_invalid_lifetime( def _metric_bound( - metric_value: int, lower: float, upper: float + metric_value: int, lower: float | int, upper: float | int ) -> electrical_components_pb2.MetricConfigBounds: """Build a `MetricConfigBounds` proto for the given raw metric int and bounds.""" return electrical_components_pb2.MetricConfigBounds( @@ -212,21 +212,56 @@ def _metric_bound( def test_metric_config_bounds_stores_unspecified_as_int() -> None: """Test UNSPECIFIED metric bounds load as plain int key 0.""" - major_issues: list[str] = [] - minor_issues: list[str] = [] message = [ _metric_bound(int(Metric.UNSPECIFIED.value), 0.0, 1.0), _metric_bound(_UNKNOWN_METRIC_INT, 2.0, 3.0), _metric_bound(int(Metric.DC_VOLTAGE.value), 4.0, 5.0), ] - parsed = _metric_config_bounds_from_proto( - message, major_issues=major_issues, minor_issues=minor_issues - ) + parsed = _metric_config_bounds_from_proto(message) assert parsed[int(Metric.UNSPECIFIED.value)] == Bounds(lower=0.0, upper=1.0) assert Metric.UNSPECIFIED not in parsed assert parsed[_UNKNOWN_METRIC_INT] == Bounds(lower=2.0, upper=3.0) assert parsed[Metric.DC_VOLTAGE] == Bounds(lower=4.0, upper=5.0) - assert not major_issues - assert any(str(_UNKNOWN_METRIC_INT) in issue for issue in minor_issues) + + +def test_metric_config_bounds_preserves_invalid_bounds() -> None: + """Invalid bounds are preserved as `InvalidBounds` entries, not skipped.""" + message = [ + _metric_bound(int(Metric.DC_VOLTAGE.value), 10.0, -10.0), + _metric_bound(int(Metric.AC_POWER_ACTIVE.value), -5.0, 5.0), + ] + + parsed = _metric_config_bounds_from_proto(message) + + invalid = parsed[Metric.DC_VOLTAGE] + assert isinstance(invalid, InvalidBounds) + assert not isinstance(invalid, Bounds) + assert invalid.lower == 10.0 + assert invalid.upper == -10.0 + assert parsed[Metric.AC_POWER_ACTIVE] == Bounds(lower=-5.0, upper=5.0) + + +def test_metric_config_bounds_absent_config_bounds_is_unbounded() -> None: + """An entry without a `config_bounds` field yields an unbounded `Bounds`.""" + entry = electrical_components_pb2.MetricConfigBounds( + metric=metrics_pb2.Metric.ValueType(int(Metric.DC_VOLTAGE.value)) + ) + entry.ClearField("config_bounds") + + parsed = _metric_config_bounds_from_proto([entry]) + + assert parsed[Metric.DC_VOLTAGE] == Bounds() + + +def test_metric_config_bounds_duplicated_metric_last_wins() -> None: + """A duplicated metric on the wire is kept as its last entry (proto3 map semantics).""" + message = [ + _metric_bound(int(Metric.DC_VOLTAGE.value), 0.0, 1.0), + _metric_bound(int(Metric.DC_VOLTAGE.value), 2.0, 3.0), + ] + + parsed = _metric_config_bounds_from_proto(message) + + assert parsed[Metric.DC_VOLTAGE] == Bounds(lower=2.0, upper=3.0) diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 8fed6c64..b04ad6e5 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -8,8 +8,16 @@ import pytest -from frequenz.client.common import UnrecognizedEnumValueError, UnspecifiedEnumValueError -from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common import ( + UnrecognizedEnumValueError, + UnspecifiedEnumValueError, +) +from frequenz.client.common.metrics import ( + Bounds, + InvalidBounds, + InvalidBoundsError, + Metric, +) from frequenz.client.common.microgrid import ( InvalidLifetime, InvalidLifetimeError, @@ -27,14 +35,19 @@ class _TestElectricalComponent(ElectricalComponent): def _make_component( - operational_lifetime: Lifetime | InvalidLifetime, + *, + operational_lifetime: Lifetime | InvalidLifetime = Lifetime(), + metric_config_bounds: dict[Metric | int, Bounds | InvalidBounds] | None = None, ) -> _TestElectricalComponent: """Build a test component with the given operational lifetime.""" + if metric_config_bounds is None: + metric_config_bounds = {} return _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", model="Test Model", + metric_config_bounds=metric_config_bounds, operational_lifetime=operational_lifetime, _provides_telemetry=True, _accepts_control=True, @@ -170,7 +183,7 @@ def test_accessors_raise_when_unrecognized() -> None: def test_get_operational_lifetime_returns_valid() -> None: """`get_operational_lifetime()` returns a valid lifetime unchanged.""" lifetime = Lifetime() - component = _make_component(lifetime) + component = _make_component(operational_lifetime=lifetime) assert component.get_operational_lifetime() is lifetime @@ -181,7 +194,7 @@ def test_get_operational_lifetime_raises_invalid() -> None: start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), ) - component = _make_component(invalid) + component = _make_component(operational_lifetime=invalid) with pytest.raises(InvalidLifetimeError) as exc_info: component.get_operational_lifetime() @@ -191,7 +204,7 @@ def test_get_operational_lifetime_raises_invalid() -> None: def test_is_operational_at_raises_for_invalid_lifetime() -> None: """`is_operational_at()` raises when the lifetime is invalid.""" component = _make_component( - InvalidLifetime( + operational_lifetime=InvalidLifetime( start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), ) @@ -204,7 +217,7 @@ def test_is_operational_at_raises_for_invalid_lifetime() -> None: def test_is_operational_now_raises_for_invalid_lifetime() -> None: """`is_operational_now()` raises when the lifetime is invalid.""" component = _make_component( - InvalidLifetime( + operational_lifetime=InvalidLifetime( start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), ) @@ -214,6 +227,72 @@ def test_is_operational_now_raises_for_invalid_lifetime() -> None: component.is_operational_now() +def test_get_metric_config_bounds_returns_valid_bounds() -> None: + """`get_metric_config_bounds` returns the configured `Bounds` for a metric.""" + bounds = Bounds(lower=-10.0, upper=10.0) + component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: bounds}) + + result = component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) + + assert result is bounds + + +def test_get_metric_config_bounds_absent_returns_unbounded() -> None: + """`get_metric_config_bounds` returns an unbounded `Bounds` for absent metrics.""" + component = _make_component(metric_config_bounds={}) + + result = component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) + + assert result == Bounds() + assert isinstance(result, Bounds) + + +def test_get_metric_config_bounds_absent_returns_default() -> None: + """`get_metric_config_bounds` returns `default` for absent metrics.""" + component = _make_component(metric_config_bounds={}) + sentinel = object() + + assert ( + component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE, default=None) is None + ) + assert ( + component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE, default=sentinel) + is sentinel + ) + + +def test_get_metric_config_bounds_present_ignores_default() -> None: + """`get_metric_config_bounds` ignores `default` when the metric has bounds.""" + bounds = Bounds(lower=-10.0, upper=10.0) + component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: bounds}) + + assert ( + component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE, default=None) + is bounds + ) + + +def test_get_metric_config_bounds_invalid_raises_error() -> None: + """`get_metric_config_bounds` raises `InvalidBoundsError` for malformed entries.""" + invalid = InvalidBounds(lower=10.0, upper=-10.0) + component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: invalid}) + + with pytest.raises(InvalidBoundsError) as exc_info: + component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE) + + assert exc_info.value.bounds is invalid + assert "AC_POWER_ACTIVE" in str(exc_info.value) + + +def test_get_metric_config_bounds_invalid_raises_despite_default() -> None: + """`default` only applies to absent metrics, not malformed ones.""" + invalid = InvalidBounds(lower=10.0, upper=-10.0) + component = _make_component(metric_config_bounds={Metric.AC_POWER_ACTIVE: invalid}) + + with pytest.raises(InvalidBoundsError): + component.get_metric_config_bounds(Metric.AC_POWER_ACTIVE, default=None) + + @pytest.mark.parametrize( "name,expected_str", [ diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index 49487044..2297df3e 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -47,9 +47,6 @@ class _ProtoConversionTestCase: is unspecified, or any other raw `int` when the status is unrecognized. """ - expected_log: tuple[str, str] | None = None - """Whether to expect a log during conversion (level, message).""" - def _assert_active(info: Microgrid, expected_active: bool | int) -> None: """Assert that ``info._active`` matches ``expected_active`` and the accessor agrees.""" @@ -94,10 +91,6 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: has_name=True, status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, expected_active=True, - expected_log=( - "WARNING", - "Found issues in microgrid: delivery_area is missing", - ), ), _ProtoConversionTestCase( name="no_location", @@ -106,7 +99,6 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: has_name=True, status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, expected_active=True, - expected_log=("WARNING", "Found issues in microgrid: location is missing"), ), _ProtoConversionTestCase( name="empty_name", @@ -123,10 +115,6 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: has_name=True, status=microgrid_pb2.MICROGRID_STATUS_UNSPECIFIED, expected_active=0, - expected_log=( - "WARNING", - "Found issues in microgrid: status is unspecified", - ), ), _ProtoConversionTestCase( name="unrecognized_status", @@ -135,10 +123,6 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: has_name=True, status=999, # Unknown status value expected_active=999, - expected_log=( - "WARNING", - "Found issues in microgrid: status is unrecognized", - ), ), ], ids=lambda case: case.name, @@ -148,12 +132,10 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: ) @patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.location_from_proto") @patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.datetime_from_proto") -# pylint: disable-next=too-many-arguments,too-many-positional-arguments,too-many-branches def test_from_proto( mock_datetime_from_proto: Mock, mock_location_from_proto: Mock, mock_delivery_area_from_proto: Mock, - caplog: pytest.LogCaptureFixture, case: _ProtoConversionTestCase, ) -> None: """Test conversion from protobuf message to Microgrid.""" @@ -201,8 +183,7 @@ def test_from_proto( proto.location.country_code = "DE" # Run the conversion - with caplog.at_level("DEBUG"): - info = microgrid_from_proto(proto) + info = microgrid_from_proto(proto) # Verify the result assert info.id == MicrogridId(1234) @@ -232,12 +213,3 @@ def test_from_proto( else: mock_location_from_proto.assert_not_called() assert info.location is None - - # Verify logging behavior - if case.expected_log: - expected_level, expected_message = case.expected_log - assert len(caplog.records) == 1 - assert caplog.records[0].levelname == expected_level - assert expected_message in caplog.records[0].message - else: - assert len(caplog.records) == 0