Skip to content

Commit b7f37dc

Browse files
test(compare): pin which config: values are kept, dropped or skipped (#5472)
Review follow-up. One EMPTY_COMPARE_CONFIG tuple now decides both the skip and the drop, rather than a tuple in one place and truthiness in the other. T27 checks the reason each config entry was skipped, and adds a mapping config, which must survive as the tariff's overrides - nothing tested that, so a check skipping or dropping every config would have passed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f41a277 commit b7f37dc

2 files changed

Lines changed: 20 additions & 9 deletions

File tree

‎apps/predbat/compare.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
# 3. Consider Octopus API key to access current tariff info and switch links
2828

2929

30+
# An empty 'config:' line means no overrides; any other non-mapping value can't be applied
31+
EMPTY_COMPARE_CONFIG = (None, "", [], {})
32+
33+
3034
def check_compare_list(compare_list):
3135
"""Split compare_list into the entries that can be compared and a message per skipped entry (#5472).
3236
@@ -58,7 +62,7 @@ def check_compare_list(compare_list):
5862
problem = "has a duplicate id '{}'".format(entry_id)
5963
elif not entry_name:
6064
problem = "('{}') requires a non-empty 'name'".format(entry_id)
61-
elif entry.get("config") not in (None, "", []) and not isinstance(entry["config"], dict):
65+
elif entry.get("config") not in EMPTY_COMPARE_CONFIG and not isinstance(entry["config"], dict):
6266
# run_all() and fetch_config() look overrides up by key; a list of them raised TypeError
6367
# there and ended the whole comparison, and any other value was never a usable override
6468
problem = "('{}') has a 'config' that isn't a set of 'setting: value' overrides".format(entry_id)
@@ -67,8 +71,7 @@ def check_compare_list(compare_list):
6771
continue
6872
seen_ids.add(entry_id)
6973
checked = dict(entry, id=entry_id, name=entry_name)
70-
if not checked.get("config"):
71-
# An empty 'config:' line (None, "", [] or {}) means no overrides, as it always has
74+
if checked.get("config") in EMPTY_COMPARE_CONFIG:
7275
checked.pop("config", None)
7376
valid.append(checked)
7477
return valid, problems

‎apps/predbat/tests/test_compare.py‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -723,10 +723,11 @@ def _mock_plan_iboost_smart():
723723
# ------------------------------------------------------------------
724724
# T27: valid_compare_list() keeps only the entries run_all() can compare -
725725
# skipping, with a warning naming the entry's position, one that isn't a
726-
# dict, has no id, repeats an earlier id or has no name - and turns a
727-
# numeric id into a string (#5472 - run_all() used to index tariff["id"]
728-
# directly, raising KeyError every cycle, and a numeric id still crashed
729-
# publish_data() building the sensor name)
726+
# dict, has no id, repeats an earlier id, has no name or has a config:
727+
# that is neither a mapping nor empty (None, "", [] and {} mean no
728+
# overrides) - and turns a numeric id into a string (#5472 - run_all()
729+
# used to index tariff["id"] directly, raising KeyError every cycle, and a
730+
# numeric id still crashed publish_data() building the sensor name)
730731
# ------------------------------------------------------------------
731732
logged = []
732733
entries = [
@@ -747,17 +748,24 @@ def _mock_plan_iboost_smart():
747748
{"id": "empty_dict_config", "name": "Empty Dict Config", "config": {}},
748749
{"id": "false_config", "name": "False Config", "config": False},
749750
{"id": "string_config", "name": "String Config", "config": "best_soc_min: 2.0"},
751+
{"id": "mapping_config", "name": "Mapping Config", "config": {"best_soc_min": 2.0}},
750752
]
751753
kept = valid_compare_list(entries, log=logged.append)
752754
kept_ids = [entry["id"] for entry in kept]
753755
skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged)
754756

755-
if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config"]:
756-
print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name' and the four empty-config entries (ids as strings), got {}".format(kept_ids))
757+
if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config", "mapping_config"]:
758+
print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name', the four empty-config entries and 'mapping_config' (ids as strings), got {}".format(kept_ids))
757759
failed += 1
758760
elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11, 16, 17]:
759761
print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6, 8 (padded duplicate id), 10 (blank name), 11, 16 and 17 (unusable config), got {}".format(logged))
760762
failed += 1
763+
elif not all("'config'" in msg for msg in logged if msg.split("compare_list entry ")[1].split(" ")[0] in ("11", "16", "17")):
764+
print("ERROR T27: entries 11, 16 and 17 should be skipped for their config, got {}".format(logged))
765+
failed += 1
766+
elif kept[7].get("config") != {"best_soc_min": 2.0}:
767+
print("ERROR T27: a mapping config: should be kept as the tariff's overrides, got {}".format(kept[7]))
768+
failed += 1
761769
elif any("config" in entry for entry in kept[3:7]):
762770
print("ERROR T27: an empty 'config:' (None, [], \"\" or {{}}) should be dropped as no overrides, got {}".format(kept[3:7]))
763771
failed += 1

0 commit comments

Comments
 (0)