Skip to content

Commit 101fb92

Browse files
Remove redundant check in BatteryPool tests
This commit removes a redundant check from the BatteryPool tests. The check was deemed unnecessary as we already use a timeout when waiting for messages, which effectively proves that messages are received quickly. The removed check had recently started failing in CI environments, on arm64 architecture with Python3.11 and pytest_max. Despite efforts to reproduce the failure locally on arm64, all tests passed successfully. The CI failure appears to be caused by an 'await' hanging rather than an issue in our codebase. Removing this redundant check should resolve the intermittent CI failures without compromising test coverage or reliability. Signed-off-by: Elzbieta Kotulska <[email protected]>
1 parent 8eabc21 commit 101fb92

File tree

1 file changed

+15
-25
lines changed

1 file changed

+15
-25
lines changed

tests/timeseries/_battery_pool/test_battery_pool.py

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ async def run_scenarios(
293293
continue
294294

295295
try:
296-
compare_messages(msg, scenario.expected_result, waiting_time_sec)
296+
compare_messages(msg, scenario.expected_result)
297297
except AssertionError as err:
298298
_logger.error("Test scenario: %d failed.", idx)
299299
raise err
@@ -386,17 +386,15 @@ def assert_dataclass(arg: Any) -> None:
386386
assert is_dataclass(arg), f"Expected dataclass, received {type(arg)}, {arg}"
387387

388388

389-
def compare_messages(msg: Any, expected_msg: Any, time_diff: float) -> None:
389+
def compare_messages(msg: Any, expected_msg: Any) -> None:
390390
"""Compare two dataclass arguments.
391391
392392
Compare if both are dataclass.
393393
Compare if all its arguments except `timestamp` are equal.
394-
Check if timestamp of the msg is not older then `time_diff`.
395394
396395
Args:
397396
msg: dataclass to compare
398397
expected_msg: expected dataclass
399-
time_diff: maximum time difference between now and the `msg`
400398
"""
401399
assert_dataclass(msg)
402400
assert_dataclass(expected_msg)
@@ -407,14 +405,10 @@ def compare_messages(msg: Any, expected_msg: Any, time_diff: float) -> None:
407405
assert "timestamp" in msg_dict
408406
assert "timestamp" in expected_dict
409407

410-
msg_timestamp = msg_dict.pop("timestamp")
408+
msg_dict.pop("timestamp")
411409
expected_dict.pop("timestamp")
412-
413410
assert msg_dict == expected_dict
414411

415-
diff = datetime.now(tz=timezone.utc) - msg_timestamp
416-
assert diff.total_seconds() < time_diff
417-
418412

419413
async def run_test_battery_status_channel( # pylint: disable=too-many-arguments
420414
battery_status_sender: Sender[ComponentPoolStatus],
@@ -450,7 +444,7 @@ async def run_test_battery_status_channel( # pylint: disable=too-many-arguments
450444
msg = await asyncio.wait_for(
451445
battery_pool_metric_receiver.receive(), timeout=waiting_time_sec
452446
)
453-
compare_messages(msg, only_first_battery_result, waiting_time_sec)
447+
compare_messages(msg, only_first_battery_result)
454448

455449
# All batteries stopped working data
456450
working -= {batteries_in_pool[0]}
@@ -475,7 +469,7 @@ async def run_test_battery_status_channel( # pylint: disable=too-many-arguments
475469
msg = await asyncio.wait_for(
476470
battery_pool_metric_receiver.receive(), timeout=waiting_time_sec
477471
)
478-
compare_messages(msg, only_first_battery_result, waiting_time_sec)
472+
compare_messages(msg, only_first_battery_result)
479473

480474
# All batteries are working again
481475
await battery_status_sender.send(
@@ -484,7 +478,7 @@ async def run_test_battery_status_channel( # pylint: disable=too-many-arguments
484478
msg = await asyncio.wait_for(
485479
battery_pool_metric_receiver.receive(), timeout=waiting_time_sec
486480
)
487-
compare_messages(msg, all_pool_result, waiting_time_sec)
481+
compare_messages(msg, all_pool_result)
488482

489483

490484
async def test_battery_pool_power(mocker: MockerFixture) -> None:
@@ -788,7 +782,7 @@ async def run_capacity_test( # pylint: disable=too-many-locals
788782
50.0
789783
), # 50% of 50 kWh + 50% of 50 kWh = 25 + 25 = 50 kWh
790784
)
791-
compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2)
785+
compare_messages(msg, expected)
792786

793787
batteries_in_pool = list(battery_pool.component_ids)
794788
scenarios: list[Scenario[Sample[Energy]]] = [
@@ -914,9 +908,7 @@ async def run_capacity_test( # pylint: disable=too-many-locals
914908
fake_time.shift(MAX_BATTERY_DATA_AGE_SEC + 0.2)
915909
msg = await asyncio.wait_for(capacity_receiver.receive(), timeout=waiting_time_sec)
916910
# the msg time difference shouldn't be bigger then the shifted time + 0.2 sec tolerance
917-
compare_messages(
918-
msg, Sample(now, Energy.from_watt_hours(21.0)), MAX_BATTERY_DATA_AGE_SEC + 0.4
919-
)
911+
compare_messages(msg, Sample(now, Energy.from_watt_hours(21.0)))
920912

921913
# All batteries stopped sending data.
922914
await streamer.stop_streaming(batteries_in_pool[0])
@@ -933,7 +925,6 @@ async def run_capacity_test( # pylint: disable=too-many-locals
933925
compare_messages(
934926
msg,
935927
Sample(datetime.now(tz=timezone.utc), Energy.from_watt_hours(21.0)),
936-
MAX_BATTERY_DATA_AGE_SEC + 0.4,
937928
)
938929

939930

@@ -980,7 +971,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
980971
timestamp=now,
981972
value=Percentage.from_percent(10.0),
982973
)
983-
compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2)
974+
compare_messages(msg, expected)
984975

985976
batteries_in_pool = list(battery_pool.component_ids)
986977
scenarios: list[Scenario[Sample[Percentage]]] = [
@@ -1055,7 +1046,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
10551046
await streamer.stop_streaming(batteries_in_pool[1])
10561047
await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
10571048
msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
1058-
compare_messages(msg, Sample(now, Percentage.from_percent(50.0)), 0.2)
1049+
compare_messages(msg, Sample(now, Percentage.from_percent(50.0)))
10591050

10601051
# All batteries stopped sending data.
10611052
await streamer.stop_streaming(batteries_in_pool[0])
@@ -1067,7 +1058,7 @@ async def run_soc_test(setup_args: SetupArgs) -> None:
10671058
latest_data = streamer.get_current_component_data(batteries_in_pool[0])
10681059
streamer.start_streaming(latest_data, sampling_rate=0.1)
10691060
msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
1070-
compare_messages(msg, Sample(now, Percentage.from_percent(50.0)), 0.2)
1061+
compare_messages(msg, Sample(now, Percentage.from_percent(50.0)))
10711062

10721063

10731064
async def run_power_bounds_test( # pylint: disable=too-many-locals
@@ -1136,7 +1127,7 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals
11361127
inclusion_bounds=Bounds(Power.from_watts(-1800), Power.from_watts(10000)),
11371128
exclusion_bounds=Bounds(Power.from_watts(-600), Power.from_watts(600)),
11381129
)
1139-
compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2)
1130+
compare_messages(msg, expected)
11401131

11411132
batteries_in_pool = list(battery_pool.component_ids)
11421133
scenarios: list[Scenario[SystemBounds]] = [
@@ -1315,7 +1306,6 @@ async def run_power_bounds_test( # pylint: disable=too-many-locals
13151306
inclusion_bounds=Bounds(Power.from_watts(-300), Power.from_watts(400)),
13161307
exclusion_bounds=Bounds(Power.from_watts(-130), Power.from_watts(130)),
13171308
),
1318-
0.2,
13191309
)
13201310

13211311
# All components stopped sending data, we can assume that power bounds are 0
@@ -1370,7 +1360,7 @@ async def run_temperature_test( # pylint: disable=too-many-locals
13701360
)
13711361
now = datetime.now(tz=timezone.utc)
13721362
expected = Sample(now, value=Temperature.from_celsius(25.0))
1373-
compare_messages(msg, expected, WAIT_FOR_COMPONENT_DATA_SEC + 0.2)
1363+
compare_messages(msg, expected)
13741364

13751365
batteries_in_pool = list(battery_pool.component_ids)
13761366
bat_0, bat_1 = batteries_in_pool
@@ -1424,7 +1414,7 @@ async def run_temperature_test( # pylint: disable=too-many-locals
14241414
await streamer.stop_streaming(bat_1)
14251415
await asyncio.sleep(MAX_BATTERY_DATA_AGE_SEC + 0.2)
14261416
msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
1427-
compare_messages(msg, Sample(now, Temperature.from_celsius(30.0)), 0.2)
1417+
compare_messages(msg, Sample(now, Temperature.from_celsius(30.0)))
14281418

14291419
# All batteries stopped sending data.
14301420
await streamer.stop_streaming(bat_0)
@@ -1436,4 +1426,4 @@ async def run_temperature_test( # pylint: disable=too-many-locals
14361426
latest_data = streamer.get_current_component_data(bat_1)
14371427
streamer.start_streaming(latest_data, sampling_rate=0.1)
14381428
msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
1439-
compare_messages(msg, Sample(now, Temperature.from_celsius(15.0)), 0.2)
1429+
compare_messages(msg, Sample(now, Temperature.from_celsius(15.0)))

0 commit comments

Comments
 (0)