Skip to content

Commit c5ff250

Browse files
committed
Don't rely on the order of a battery set in tests
These tests rely on the order in which components are returned from the component graph using `all_batteries[:2]` to get the first 2 batteries. But the component graph returns a `set`, which doesn't have any ordering guarantees, so if the Python implementation changes, the hashing can change and this resulting order could be different, breaking the tests. This is why we are now specifying IDs to use explicitly (these are the IDs that were used when the tests were written). This can also change in the future if generator of mock components changes, as it might produce different IDs, so this solution is also not bullet-proof. Signed-off-by: Leandro Lucarella <[email protected]>
1 parent ee89173 commit c5ff250

File tree

1 file changed

+15
-5
lines changed

1 file changed

+15
-5
lines changed

tests/timeseries/_battery_pool/test_battery_pool.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,21 @@ async def setup_batteries_pool(mocker: MockerFixture) -> AsyncIterator[SetupArgs
198198
# the scope of this tests. This tests should cover BatteryPool only.
199199
# We use our own battery status channel, where we easily control set of working
200200
# batteries.
201-
all_batteries = list(get_components(mock_microgrid, ComponentCategory.BATTERY))
201+
all_batteries = get_components(mock_microgrid, ComponentCategory.BATTERY)
202202

203-
battery_pool = microgrid.new_battery_pool(
204-
priority=5, component_ids=set(all_batteries[:2])
205-
)
203+
# This is a hack because these tests used to rely on the order in which components
204+
# are returned from the component graph using `all_batteries[:2]` to get the first 2
205+
# batteries. But the component graph returns a set, which doesn't have any ordering
206+
# guarantees, so if the Python implementation changes, the hashing can change and
207+
# this resulting order could be different, breaking the tests. This is why we are
208+
# now specifying IDs to use explicitly (these are the IDs that were used when the
209+
# tests were written). This can also change in the future if generator of mock
210+
# components changes, as it might produce different IDs, so this solution is also
211+
# not bullet-proof.
212+
assert 8 in all_batteries
213+
assert 11 in all_batteries
214+
215+
battery_pool = microgrid.new_battery_pool(priority=5, component_ids=set([8, 11]))
206216

207217
dp = microgrid._data_pipeline._DATA_PIPELINE
208218
assert dp is not None
@@ -1125,7 +1135,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals
11251135
)
11261136
compare_messages(msg, expected)
11271137

1128-
batteries_in_pool = list(battery_pool.component_ids)
1138+
batteries_in_pool = list(sorted(battery_pool.component_ids))
11291139
scenarios: list[Scenario[SystemBounds]] = [
11301140
Scenario(
11311141
next(iter(bat_invs_map[batteries_in_pool[0]])),

0 commit comments

Comments
 (0)