Skip to content

Commit ee89173

Browse files
committed
Sort component IDs in exceptions
These component IDs are stored in sets, which don't have a order guarantee, and looking at component IDs in exceptions with random order makes it hard to identify if a particular component is in the list or not. This also makes it easier to write tests that are not flaky or depend on the component's order. To improve the exceptions also `", ".join()` is used to get less noise in the exception message (like `frozenset({4, 2, 6})` vs `2, 4, 5`). This makes exceptions easier to read because it is easier to look for a component ID Signed-off-by: Leandro Lucarella <[email protected]>
1 parent abe74d7 commit ee89173

File tree

3 files changed

+11
-10
lines changed

3 files changed

+11
-10
lines changed

src/frequenz/sdk/microgrid/_power_managing/_matryoshka.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,9 @@ def _validate_component_ids(
144144

145145
for bucket in self._component_buckets:
146146
if any(component_id in bucket for component_id in component_ids):
147+
comp_ids = ", ".join(map(str, sorted(component_ids)))
147148
raise NotImplementedError(
148-
f"PowerManagingActor: component IDs {component_ids} are already"
149+
f"PowerManagingActor: component IDs {comp_ids} are already"
149150
" part of another bucket. Overlapping buckets are not"
150151
" yet supported."
151152
)

src/frequenz/sdk/microgrid/component_graph.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -915,14 +915,14 @@ def _validate_graph(self) -> None:
915915
# This check doesn't seem to have much sense, it only search for nodes without
916916
# data associated with them. We leave it here for now, but we should consider
917917
# removing it in the future.
918-
undefined: list[int] = [
918+
if undefined := [
919919
node[0] for node in self._graph.nodes(data=True) if len(node[1]) == 0
920-
]
921-
922-
if len(undefined) > 0:
920+
]:
921+
undefined_str = ", ".join(map(str, sorted(undefined)))
923922
raise InvalidGraphError(
924-
f"Missing definition for graph components: {undefined}"
923+
f"Missing definition for graph components: {undefined_str}"
925924
)
925+
926926
# should be true as a consequence of checks above
927927
if sum(1 for _ in self.components()) <= 0:
928928
raise InvalidGraphError("Graph must have a least one component!")

tests/actor/_power_managing/test_matryoshka.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,8 @@ async def test_matryoshka_drop_old_proposals() -> None:
435435
with pytest.raises(
436436
NotImplementedError,
437437
match=re.escape(
438-
"PowerManagingActor: component IDs frozenset({8, 5}) are already "
439-
+ "part of another bucket. Overlapping buckets are not yet supported."
438+
"PowerManagingActor: component IDs 5, 8 are already part of "
439+
"another bucket. Overlapping buckets are not yet supported."
440440
),
441441
):
442442
tester.tgt_power(
@@ -506,8 +506,8 @@ def ensure_overlapping_bucket_request_fails() -> None:
506506
with pytest.raises(
507507
NotImplementedError,
508508
match=re.escape(
509-
"PowerManagingActor: component IDs frozenset({8, 5}) are already "
510-
+ "part of another bucket. Overlapping buckets are not yet supported."
509+
"PowerManagingActor: component IDs 5, 8 are already part of "
510+
"another bucket. Overlapping buckets are not yet supported."
511511
),
512512
):
513513
tester.tgt_power(

0 commit comments

Comments
 (0)