From f92f8fa2e8a5772a7cdfe51b4aa8ee37daf0a0f0 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 15:15:58 +0100 Subject: [PATCH 1/9] fix(compare): skip a compare_list entry with no id instead of crashing (#5472) Compare.run_all() indexed tariff["id"] directly, so a compare_list entry missing its id line raised KeyError every 5-minute cycle - and because the crash skipped predbat.py's compare_active reset, the comparison (and the crash) kept re-triggering until the config was fixed. The id lookup and its warning are now in their own get_tariff_id() method, so the guard can be tested directly rather than only by inspecting run_all()'s source text (code review for #5472 found the source-inspection test alone would pass a guard that checked the wrong field, e.g. tariff.get("name")). Co-Authored-By: Claude Sonnet 5 --- apps/predbat/compare.py | 17 +++++++++-- apps/predbat/predbat.py | 9 ++++-- apps/predbat/tests/test_compare.py | 49 ++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index e15fb1d78..d7d6a8e35 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -592,6 +592,14 @@ def recompute_car_charging(self, car_charging_slots): if my_predbat.car_charging_planned[car_n] and my_predbat.car_charging_exclusive[car_n]: break + def get_tariff_id(self, tariff): + """Return a compare_list entry's id, or None and a warning if it has none (#5472).""" + tariff_id = tariff.get("id") + if not tariff_id: + self.log("Warn: Compare tariff {} has no id, skipping - add an 'id:' line to this compare_list entry".format(tariff.get("name", ""))) + return None + return tariff_id + def run_all(self, debug=False, fetch_sensor=True): """ Compare a comparison in prices across multiple energy tariffs and report results @@ -672,7 +680,10 @@ def run_all(self, debug=False, fetch_sensor=True): try: for tariff in compare_list: - prior = results.get(tariff["id"], {}) + tariff_id = self.get_tariff_id(tariff) + if not tariff_id: + continue + prior = results.get(tariff_id, {}) if prior: prior_date = prior.get("date", "")[:10] if prior_date == today_date: @@ -684,11 +695,11 @@ def run_all(self, debug=False, fetch_sensor=True): else: # First ever run for this tariff: start from actual midnight SoC start_soc = soc_midnight_fallback - self.log("Compare tariff {} starting SoC: {:.2f} kWh".format(tariff.get("id", ""), start_soc)) + self.log("Compare tariff {} starting SoC: {:.2f} kWh".format(tariff_id, start_soc)) try: result_data = self.run_single(tariff, rate_import_base, rate_export_base, end_record, debug=debug, fetch_sensor=fetch_sensor, car_charging_slots=save_car_charging_slots, start_soc=start_soc, io_adjusted_base=io_adjusted_base) if result_data is not None: - results[tariff["id"]] = result_data + results[tariff_id] = result_data finally: # Restore hardware settings after each tariff so overrides don't bleed into the next tariff; # in a finally so a raised run_single() can't leave compare values applied to the live instance diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index 93508ee50..1d2efe57f 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -1430,10 +1430,13 @@ def update_pred(self, scheduled=True): if self.comparison: if (scheduled and self.minutes_now < RUN_EVERY) or self.get_arg("compare_active", False): - # Compare tariffs either when triggered or daily at midnight + # Compare tariffs either when triggered or daily at midnight; the reset must run even if + # run_all() raises, otherwise a crashed comparison re-triggers itself every cycle (#5472) self.expose_config("compare_active", True) - self.comparison.run_all() - self.expose_config("compare_active", False) + try: + self.comparison.run_all() + finally: + self.expose_config("compare_active", False) else: # Otherwise just update HA sensors to prevent then expiring self.comparison.publish_only() diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index ac7629f31..672ee4778 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -717,6 +717,55 @@ def _mock_plan_iboost_smart(): else: print("PASS T26: only a tariff reusing the live import rates keeps the live dispatch markers") + # ------------------------------------------------------------------ + # T27: get_tariff_id() returns None (with a warning) for a compare_list + # entry with no id, and the real id string otherwise (#5472 - run_all() + # used to index tariff["id"] directly, raising KeyError every cycle; a + # crash there also left compare_active stuck on, re-triggering the + # comparison - and the crash - every 5-minute cycle) + # ------------------------------------------------------------------ + cmp, pb = _make_compare() + logged = [] + cmp.log = lambda msg: logged.append(msg) + + no_id_result = cmp.get_tariff_id({"name": "No Id Tariff"}) + has_id_result = cmp.get_tariff_id({"id": "has_id", "name": "Has Id Tariff"}) + + if no_id_result is not None: + print("ERROR T27: get_tariff_id() should return None for an entry with no id, got {}".format(no_id_result)) + failed += 1 + elif not any("No Id Tariff" in msg for msg in logged): + print("ERROR T27: get_tariff_id() should log a warning naming the id-less entry, got {}".format(logged)) + failed += 1 + elif has_id_result != "has_id": + print("ERROR T27: get_tariff_id() should return the real id for an entry that has one, got {}".format(has_id_result)) + failed += 1 + else: + print("PASS T27: get_tariff_id() returns None and warns for an id-less entry, the real id otherwise") + + # ------------------------------------------------------------------ + # T28: run_all() is actually wired to call get_tariff_id() inside its tariff + # loop, rather than T27 only proving the helper works in isolation - + # otherwise run_all() could still index tariff["id"] directly (#5472) + # ------------------------------------------------------------------ + run_all_source = inspect.getsource(Compare.run_all) + loop_idx = run_all_source.find("for tariff in compare_list:") + guard_idx = run_all_source.find("self.get_tariff_id(tariff)", loop_idx + 1) if loop_idx >= 0 else -1 + continue_idx = run_all_source.find("continue", guard_idx + 1) if guard_idx >= 0 else -1 + first_direct_index_idx = run_all_source.find('tariff["id"]', loop_idx + 1) if loop_idx >= 0 else -1 + + if loop_idx < 0 or guard_idx < 0 or continue_idx < 0: + print("ERROR T28: could not locate a call to get_tariff_id() inside run_all()'s tariff loop - test may need updating") + failed += 1 + elif not (loop_idx < guard_idx < continue_idx): + print("ERROR T28: run_all() does not skip an id-less tariff before using its id") + failed += 1 + elif first_direct_index_idx != -1 and first_direct_index_idx < continue_idx: + print("ERROR T28: run_all() still indexes tariff[\"id\"] directly before the id guard takes effect") + failed += 1 + else: + print("PASS T28: run_all() calls get_tariff_id() inside its tariff loop before using the id") + if failed: print("**** compare tests FAILED: {} errors ****\n".format(failed)) else: From 13521cc70f1bba084187a92e164ef3db2ddc1a7b Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 22:06:11 +0100 Subject: [PATCH 2/9] test(compare): behavioural tests for the id-less skip and compare_active reset (#5472) Replace T28's source-text check with a run_all() run against the real fixture (run_single/save_yaml stubbed) showing only the valid tariff is compared and persisted. Extract the compare_active set/clear into PredBat.run_comparison() so T29 can make run_all() raise and check the flag is cleared, without driving all of update_pred(). Both tests fail with the fixes reverted. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/predbat.py | 18 ++++--- apps/predbat/tests/test_compare.py | 87 ++++++++++++++++++++++++------ 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index 1d2efe57f..529ac9431 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -1070,6 +1070,15 @@ def update_car_manual_soc(self): soc_next = min(soc_next, self.car_charging_limit[car_n]) self.expose_config("car_charging_manual_soc_kwh" + car_postfix, dp3(soc_next)) + def run_comparison(self): + """Run the tariff comparison, clearing compare_active afterwards even if it raises (#5472).""" + # Without the finally, a crashed comparison leaves compare_active on and re-triggers itself every cycle + self.expose_config("compare_active", True) + try: + self.comparison.run_all() + finally: + self.expose_config("compare_active", False) + def update_pred(self, scheduled=True): """ Update the prediction state, everything is called from here right now @@ -1430,13 +1439,8 @@ def update_pred(self, scheduled=True): if self.comparison: if (scheduled and self.minutes_now < RUN_EVERY) or self.get_arg("compare_active", False): - # Compare tariffs either when triggered or daily at midnight; the reset must run even if - # run_all() raises, otherwise a crashed comparison re-triggers itself every cycle (#5472) - self.expose_config("compare_active", True) - try: - self.comparison.run_all() - finally: - self.expose_config("compare_active", False) + # Compare tariffs either when triggered or daily at midnight + self.run_comparison() else: # Otherwise just update HA sensors to prevent then expiring self.comparison.publish_only() diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index 672ee4778..fc4888670 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -19,6 +19,7 @@ """ import inspect +from unittest.mock import MagicMock, patch from compare import Compare from const import MINUTE_WATT @@ -744,27 +745,83 @@ def _mock_plan_iboost_smart(): print("PASS T27: get_tariff_id() returns None and warns for an id-less entry, the real id otherwise") # ------------------------------------------------------------------ - # T28: run_all() is actually wired to call get_tariff_id() inside its tariff - # loop, rather than T27 only proving the helper works in isolation - - # otherwise run_all() could still index tariff["id"] directly (#5472) + # T28: run_all() skips an id-less compare_list entry and still compares the + # valid one after it - run against the real fixture with the expensive + # collaborators stubbed, so only the loop's own handling is exercised (#5472) # ------------------------------------------------------------------ - run_all_source = inspect.getsource(Compare.run_all) - loop_idx = run_all_source.find("for tariff in compare_list:") - guard_idx = run_all_source.find("self.get_tariff_id(tariff)", loop_idx + 1) if loop_idx >= 0 else -1 - continue_idx = run_all_source.find("continue", guard_idx + 1) if guard_idx >= 0 else -1 - first_direct_index_idx = run_all_source.find('tariff["id"]', loop_idx + 1) if loop_idx >= 0 else -1 + cmp = Compare.__new__(Compare) + cmp.pb = my_predbat + cmp.log = lambda msg: None + cmp.comparisons = {} + cmp.live_saving_state = None + run_single_ids = [] + saved_comparisons = [] + cmp.run_single = lambda tariff, *args, **kwargs: run_single_ids.append(tariff.get("id")) or {"date": "2026-10-11", "soc": 1.0, "soc_start": 1.0} + cmp.select_best = lambda compare_list, results: None + cmp.save_yaml = lambda: saved_comparisons.append(dict(cmp.comparisons)) + cmp.publish_data = lambda: None + compare_list = [{"name": "No Id Tariff"}, {"id": "valid", "name": "Valid Tariff"}] + real_get_arg = my_predbat.get_arg + + def fake_get_arg(key, default=None, *args, **kwargs): + """Return the test compare_list, deferring every other key to the real get_arg.""" + if key == "compare_list": + return compare_list + return real_get_arg(key, default, *args, **kwargs) + + try: + with patch.object(my_predbat, "get_arg", side_effect=fake_get_arg): + cmp.run_all(fetch_sensor=False) + run_all_error = None + except Exception as exc: + run_all_error = exc + + if run_all_error is not None: + print("ERROR T28: run_all() raised {!r} on a compare_list with an id-less entry".format(run_all_error)) + failed += 1 + elif run_single_ids != ["valid"]: + print("ERROR T28: only the valid tariff should reach run_single(), got {}".format(run_single_ids)) + failed += 1 + elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid"}: + print("ERROR T28: only the valid tariff's result should be persisted, got {}".format(saved_comparisons)) + failed += 1 + else: + print("PASS T28: run_all() skips an id-less entry and compares and persists the valid one") + + # ------------------------------------------------------------------ + # T29: run_comparison() clears compare_active even when run_all() raises - + # otherwise the flag stays on and the crashed comparison re-triggers + # itself every 5-minute cycle (#5472) + # ------------------------------------------------------------------ + exposed = [] + real_expose_config = my_predbat.expose_config + + def record_expose_config(name, value, *args, **kwargs): + """Record compare_active writes, then apply them for real.""" + if name == "compare_active": + exposed.append(value) + return real_expose_config(name, value, *args, **kwargs) + + raising_comparison = MagicMock() + raising_comparison.run_all.side_effect = KeyError("id") + raised = False + with patch.object(my_predbat, "comparison", raising_comparison), patch.object(my_predbat, "expose_config", side_effect=record_expose_config): + try: + my_predbat.run_comparison() + except KeyError: + raised = True - if loop_idx < 0 or guard_idx < 0 or continue_idx < 0: - print("ERROR T28: could not locate a call to get_tariff_id() inside run_all()'s tariff loop - test may need updating") + if not raised: + print("ERROR T29: run_comparison() should let run_all()'s exception propagate") failed += 1 - elif not (loop_idx < guard_idx < continue_idx): - print("ERROR T28: run_all() does not skip an id-less tariff before using its id") + elif exposed != [True, False]: + print("ERROR T29: compare_active should be set then cleared despite the exception, got {}".format(exposed)) failed += 1 - elif first_direct_index_idx != -1 and first_direct_index_idx < continue_idx: - print("ERROR T28: run_all() still indexes tariff[\"id\"] directly before the id guard takes effect") + elif my_predbat.get_arg("compare_active", False): + print("ERROR T29: compare_active is still on after run_all() raised") failed += 1 else: - print("PASS T28: run_all() calls get_tariff_id() inside its tariff loop before using the id") + print("PASS T29: run_comparison() clears compare_active even when run_all() raises") if failed: print("**** compare tests FAILED: {} errors ****\n".format(failed)) From 1c9b840baf28653660db3ede1b5a6dcf70c72337 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 22:56:22 +0100 Subject: [PATCH 3/9] fix(compare): validate compare_list once, skipping every malformed entry (#5472) get_tariff_id() only caught a missing id. A non-dict entry, a duplicate id or a nameless entry still misbehaved in run_all(), and an unquoted numeric id (YAML int) still crashed publish_data() building the sensor name - every cycle, via publish_only(). Replace it with module-level compare_entry_problem()/valid_compare_list(), which run_all(), publish_only() and load_yaml() use to drop bad entries (warning with the entry's position) and normalise ids to strings. load_yaml() also str()s stored keys so a numeric key saved earlier can't crash startup. T29 now restores compare_active on the shared fixture. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/compare.py | 55 +++++++++++---- apps/predbat/tests/test_compare.py | 106 +++++++++++++++++++++-------- 2 files changed, 117 insertions(+), 44 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index d7d6a8e35..9c2e9842a 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -27,6 +27,39 @@ # 3. Consider Octopus API key to access current tariff info and switch links +def compare_entry_problem(entry, seen_ids): + """Return why a compare_list entry can't be compared, or None if it can (#5472).""" + if not isinstance(entry, dict): + return "must be a dictionary with an id and a name" + raw_id = entry.get("id") + entry_id = "" if raw_id is None else str(raw_id) + if not entry_id.strip(): + return "requires a non-empty 'id'" + if entry_id in seen_ids: + return "has a duplicate id '{}'".format(entry_id) + if not entry.get("name"): + return "('{}') requires a non-empty 'name'".format(entry_id) + return None + + +def valid_compare_list(compare_list, log=None): + """Return the compare_list entries that can be compared, with ids as strings, logging each one skipped (#5472).""" + valid = [] + seen_ids = set() + for index, entry in enumerate(compare_list or []): + problem = compare_entry_problem(entry, seen_ids) + if problem: + if log: + log("Warn: Compare, compare_list entry {} {}, skipping it".format(index + 1, problem)) + continue + # Results, sensor names and the web page are all keyed by the id as a string, so an + # unquoted numeric id in apps.yaml must not reach them as an int + entry_id = str(entry["id"]) + seen_ids.add(entry_id) + valid.append(dict(entry, id=entry_id)) + return valid + + class Compare: """Tariff comparison engine. @@ -483,12 +516,14 @@ def load_yaml(self): try: data = yaml.safe_load(f) if data: - self.comparisons = data.get("comparisons", {}) + # str() keys: a numeric id saved before ids were normalised (#5472) would + # otherwise break publish_data() and never match its compare_list entry again + self.comparisons = {str(tariff_id): result for tariff_id, result in (data.get("comparisons") or {}).items()} except yaml.YAMLError as exc: self.log("Error loading comparisons: {}".format(exc)) if self.comparisons: - compare_list = self.pb.get_arg("compare_list", []) + compare_list = valid_compare_list(self.pb.get_arg("compare_list", [])) self.select_best(compare_list, self.comparisons) self.publish_data() @@ -539,7 +574,7 @@ def publish_only(self): """ Update HA sensors only """ - compare_list = self.pb.get_arg("compare_list", []) + compare_list = valid_compare_list(self.pb.get_arg("compare_list", [])) if not compare_list: return @@ -592,20 +627,12 @@ def recompute_car_charging(self, car_charging_slots): if my_predbat.car_charging_planned[car_n] and my_predbat.car_charging_exclusive[car_n]: break - def get_tariff_id(self, tariff): - """Return a compare_list entry's id, or None and a warning if it has none (#5472).""" - tariff_id = tariff.get("id") - if not tariff_id: - self.log("Warn: Compare tariff {} has no id, skipping - add an 'id:' line to this compare_list entry".format(tariff.get("name", ""))) - return None - return tariff_id - def run_all(self, debug=False, fetch_sensor=True): """ Compare a comparison in prices across multiple energy tariffs and report results take care not to destroy the state of the system for the primary settings """ - compare_list = self.pb.get_arg("compare_list", []) + compare_list = valid_compare_list(self.pb.get_arg("compare_list", []), log=self.log) if not compare_list: return @@ -680,9 +707,7 @@ def run_all(self, debug=False, fetch_sensor=True): try: for tariff in compare_list: - tariff_id = self.get_tariff_id(tariff) - if not tariff_id: - continue + tariff_id = tariff["id"] prior = results.get(tariff_id, {}) if prior: prior_date = prior.get("date", "")[:10] diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index fc4888670..f8e906057 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -19,9 +19,11 @@ """ import inspect +import os +import tempfile from unittest.mock import MagicMock, patch -from compare import Compare +from compare import Compare, valid_compare_list from const import MINUTE_WATT @@ -719,30 +721,41 @@ def _mock_plan_iboost_smart(): print("PASS T26: only a tariff reusing the live import rates keeps the live dispatch markers") # ------------------------------------------------------------------ - # T27: get_tariff_id() returns None (with a warning) for a compare_list - # entry with no id, and the real id string otherwise (#5472 - run_all() - # used to index tariff["id"] directly, raising KeyError every cycle; a - # crash there also left compare_active stuck on, re-triggering the - # comparison - and the crash - every 5-minute cycle) + # T27: valid_compare_list() keeps only the entries run_all() can compare - + # skipping, with a warning naming the entry's position, one that isn't a + # dict, has no id, repeats an earlier id or has no name - and turns a + # numeric id into a string (#5472 - run_all() used to index tariff["id"] + # directly, raising KeyError every cycle, and a numeric id still crashed + # publish_data() building the sensor name) # ------------------------------------------------------------------ - cmp, pb = _make_compare() logged = [] - cmp.log = lambda msg: logged.append(msg) - - no_id_result = cmp.get_tariff_id({"name": "No Id Tariff"}) - has_id_result = cmp.get_tariff_id({"id": "has_id", "name": "Has Id Tariff"}) - - if no_id_result is not None: - print("ERROR T27: get_tariff_id() should return None for an entry with no id, got {}".format(no_id_result)) - failed += 1 - elif not any("No Id Tariff" in msg for msg in logged): - print("ERROR T27: get_tariff_id() should log a warning naming the id-less entry, got {}".format(logged)) - failed += 1 - elif has_id_result != "has_id": - print("ERROR T27: get_tariff_id() should return the real id for an entry that has one, got {}".format(has_id_result)) + entries = [ + "not_a_dict", + {"name": "No Id Tariff"}, + {"id": " ", "name": "Blank Id Tariff"}, + {"id": "agile", "name": "Agile"}, + {"id": "agile", "name": "Agile Again"}, + {"id": "no_name"}, + {"id": 2024, "name": "Numeric Id Tariff"}, + ] + kept = valid_compare_list(entries, log=logged.append) + kept_ids = [entry["id"] for entry in kept] + skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) + + if kept_ids != ["agile", "2024"]: + print("ERROR T27: valid_compare_list() should keep only 'agile' and '2024' (as a string), got {}".format(kept_ids)) + failed += 1 + elif skipped_positions != [1, 2, 3, 5, 6]: + print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5 and 6, got {}".format(logged)) + failed += 1 + elif kept[0] is entries[3] or entries[6]["id"] != 2024: + print("ERROR T27: valid_compare_list() must return copies, not rewrite the user's compare_list in place") + failed += 1 + elif valid_compare_list(None) != [] or valid_compare_list([{"id": 0, "name": "Zero"}]) != [{"id": "0", "name": "Zero"}]: + print("ERROR T27: valid_compare_list() should accept a missing list and an id of 0") failed += 1 else: - print("PASS T27: get_tariff_id() returns None and warns for an id-less entry, the real id otherwise") + print("PASS T27: valid_compare_list() skips malformed entries with a positioned warning and normalises ids to strings") # ------------------------------------------------------------------ # T28: run_all() skips an id-less compare_list entry and still compares the @@ -760,7 +773,7 @@ def _mock_plan_iboost_smart(): cmp.select_best = lambda compare_list, results: None cmp.save_yaml = lambda: saved_comparisons.append(dict(cmp.comparisons)) cmp.publish_data = lambda: None - compare_list = [{"name": "No Id Tariff"}, {"id": "valid", "name": "Valid Tariff"}] + compare_list = [{"name": "No Id Tariff"}, "not_a_dict", {"id": "valid", "name": "Valid Tariff"}, {"id": "valid", "name": "Duplicate Tariff"}, {"id": 2024, "name": "Numeric Id Tariff"}] real_get_arg = my_predbat.get_arg def fake_get_arg(key, default=None, *args, **kwargs): @@ -777,16 +790,16 @@ def fake_get_arg(key, default=None, *args, **kwargs): run_all_error = exc if run_all_error is not None: - print("ERROR T28: run_all() raised {!r} on a compare_list with an id-less entry".format(run_all_error)) + print("ERROR T28: run_all() raised {!r} on a compare_list with malformed entries".format(run_all_error)) failed += 1 - elif run_single_ids != ["valid"]: - print("ERROR T28: only the valid tariff should reach run_single(), got {}".format(run_single_ids)) + elif run_single_ids != ["valid", "2024"]: + print("ERROR T28: only the valid tariffs should reach run_single(), ids as strings, got {}".format(run_single_ids)) failed += 1 - elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid"}: - print("ERROR T28: only the valid tariff's result should be persisted, got {}".format(saved_comparisons)) + elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid", "2024"}: + print("ERROR T28: only the valid tariffs' results should be persisted, keyed by string id, got {}".format(saved_comparisons)) failed += 1 else: - print("PASS T28: run_all() skips an id-less entry and compares and persists the valid one") + print("PASS T28: run_all() skips malformed entries and compares and persists the valid ones by string id") # ------------------------------------------------------------------ # T29: run_comparison() clears compare_active even when run_all() raises - @@ -802,6 +815,7 @@ def record_expose_config(name, value, *args, **kwargs): exposed.append(value) return real_expose_config(name, value, *args, **kwargs) + prior_compare_active = my_predbat.get_arg("compare_active", False) raising_comparison = MagicMock() raising_comparison.run_all.side_effect = KeyError("id") raised = False @@ -810,6 +824,9 @@ def record_expose_config(name, value, *args, **kwargs): my_predbat.run_comparison() except KeyError: raised = True + compare_active_after = my_predbat.get_arg("compare_active", False) + # Shared fixture: put the flag back as the suite had it, so test order can't matter (#5079) + my_predbat.expose_config("compare_active", prior_compare_active) if not raised: print("ERROR T29: run_comparison() should let run_all()'s exception propagate") @@ -817,12 +834,43 @@ def record_expose_config(name, value, *args, **kwargs): elif exposed != [True, False]: print("ERROR T29: compare_active should be set then cleared despite the exception, got {}".format(exposed)) failed += 1 - elif my_predbat.get_arg("compare_active", False): + elif compare_active_after: print("ERROR T29: compare_active is still on after run_all() raised") failed += 1 else: print("PASS T29: run_comparison() clears compare_active even when run_all() raises") + # ------------------------------------------------------------------ + # T30: load_yaml() turns a numeric result key saved before ids were + # normalised back into a string, so publish_data() can build the + # sensor name and the result still matches its compare_list entry (#5472) + # ------------------------------------------------------------------ + cmp, pb = _make_compare() + published = [] + pb.get_arg = lambda key, default=None: [{"id": 2024, "name": "Numeric Id Tariff"}] if key == "compare_list" else default + cmp.dashboard_item = lambda entity_id, **kwargs: published.append(entity_id) + with tempfile.TemporaryDirectory() as temp_dir: + with open(os.path.join(temp_dir, "comparisons.yaml"), "w") as f: + f.write("comparisons:\n 2024:\n name: Numeric Id Tariff\n cost: 1.0\n metric: 1.0\n") + cmp.config_root = temp_dir + try: + cmp.load_yaml() + load_error = None + except Exception as exc: + load_error = exc + + if load_error is not None: + print("ERROR T30: load_yaml() raised {!r} on a numeric result key".format(load_error)) + failed += 1 + elif list(cmp.comparisons.keys()) != ["2024"] or not cmp.comparisons["2024"].get("best"): + print("ERROR T30: the numeric key should load as '2024' and be matched as the best tariff, got {}".format(cmp.comparisons)) + failed += 1 + elif published != ["predbat.compare_tariff_2024"]: + print("ERROR T30: expected sensor predbat.compare_tariff_2024 to be published, got {}".format(published)) + failed += 1 + else: + print("PASS T30: load_yaml() normalises a numeric result key to a string and publishes it") + if failed: print("**** compare tests FAILED: {} errors ****\n".format(failed)) else: From 9a397190cfe7b2c7a68909f7b38deedba649d1a4 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:00:56 +0100 Subject: [PATCH 4/9] fix(web): share compare.py's compare_list rules in the editor and compare page (#5472) _validate_compare_list() now applies compare_entry_problem(), so the apps.yaml editor refuses exactly what a comparison would skip - now including a blank id and a numeric id clashing with the same id quoted. html_compare() reads valid_compare_list(), so a malformed entry no longer crashes the page and a numeric id finds its string-keyed result. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/tests/test_web_apps_edit.py | 16 +++++++++++++++ apps/predbat/tests/test_web_functions.py | 17 ++++++++++++++++ apps/predbat/web.py | 25 ++++++++++-------------- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/apps/predbat/tests/test_web_apps_edit.py b/apps/predbat/tests/test_web_apps_edit.py index 4cce66edf..c9864a3a0 100644 --- a/apps/predbat/tests/test_web_apps_edit.py +++ b/apps/predbat/tests/test_web_apps_edit.py @@ -380,6 +380,22 @@ def run_web_apps_edit_tests(my_predbat): print(" ERROR: expected the duplicate id to be refused, got: {}".format(result)) failed += 1 + # --------------------------------------------------------------------- + print("Test: a compare profile with a blank id is refused (#5472)") + web_interface = _reset_fixture(my_predbat) + result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", 'name: Blank\nid: " "\n')}) + if result.get("success"): + print(" ERROR: expected the blank id to be refused, got: {}".format(result)) + failed += 1 + + # --------------------------------------------------------------------- + print("Test: a numeric compare profile id clashing with the same id quoted is refused, since both publish one sensor (#5472)") + web_interface = _reset_fixture(my_predbat) + result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", "name: Numeric\nid: 7\n"), "compare_list[]#2": _add("compare_list[]", 'name: Quoted\nid: "7"\n')}) + if result.get("success"): + print(" ERROR: expected the numeric/quoted id clash to be refused, got: {}".format(result)) + failed += 1 + # --------------------------------------------------------------------- print("Test: deleting a compare profile's id is refused, since results are indexed by it") web_interface = _reset_fixture(my_predbat) diff --git a/apps/predbat/tests/test_web_functions.py b/apps/predbat/tests/test_web_functions.py index 45c0a8392..0ba777033 100644 --- a/apps/predbat/tests/test_web_functions.py +++ b/apps/predbat/tests/test_web_functions.py @@ -10,6 +10,7 @@ import asyncio import os +from unittest.mock import MagicMock, patch from web import WebInterface, is_data_numerical from web_helper import get_plan_renderer_js @@ -339,6 +340,22 @@ def run_compare_empty_state_tests(my_predbat, web): print(f" ERROR: expected the genuine loading message when compare_list is set but not yet computed") failed += 1 + # ------------------------------------------------------------------------- + print("Test: a numeric compare id finds its string-keyed result, and a malformed entry doesn't break the page (#5472)") + my_predbat.args["compare_list"] = ["not_a_dict", {"name": "No Id"}, {"id": 2024, "name": "Numeric Id Tariff"}] + fake_comparison = MagicMock() + fake_comparison.get_comparison.side_effect = lambda tariff_id: {"entity_id": "predbat.compare_tariff_2024", "html": "NUMERIC_ID_PLAN"} if tariff_id == "2024" else {} + try: + with patch.object(my_predbat, "comparison", fake_comparison): + text = asyncio.run(web.html_compare(None)).text + except Exception as exc: + text = "" + print(f" ERROR: html_compare() raised {exc!r} on a compare_list with malformed entries") + failed += 1 + if text and "NUMERIC_ID_PLAN" not in text: + print(f" ERROR: expected the plan stored under '2024' to be shown for the numeric id") + failed += 1 + my_predbat.args = original_args print("**** Compare empty state tests completed ****") diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 613c33289..11efec799 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -96,6 +96,7 @@ from predbat import THIS_VERSION_DISPLAY from component_base import ComponentBase from config import APPS_SCHEMA +from compare import compare_entry_problem, valid_compare_list import debug_history from web_annual import AnnualPage from web_chat import WebChat @@ -4081,23 +4082,18 @@ def _update_nested_yaml_value(self, data, path, value): def _validate_compare_list(self, compare_list): """ Check every compare_list profile still has the unique id and non-empty name that - compare.py indexes results by, raising ValueError if a batch has left one without + compare.py indexes results by, raising ValueError if a batch has left one without. + Uses compare.py's own rules, so the editor refuses exactly what a comparison would skip. """ if not compare_list: return seen_ids = set() - for entry in compare_list: - if not isinstance(entry, dict): - raise ValueError("Each compare_list entry must be a dictionary with an id and a name") - entry_id = entry.get("id") - if not entry_id: - raise ValueError("Each compare_list entry requires a non-empty 'id'") - if entry_id in seen_ids: - raise ValueError(f"Duplicate compare_list id '{entry_id}'") - seen_ids.add(entry_id) - if not entry.get("name"): - raise ValueError(f"compare_list entry '{entry_id}' requires a non-empty 'name'") + for index, entry in enumerate(compare_list): + problem = compare_entry_problem(entry, seen_ids) + if problem: + raise ValueError("compare_list entry {} {}".format(index + 1, problem)) + seen_ids.add(str(entry["id"])) async def html_apps_post(self, request): """ @@ -4452,7 +4448,8 @@ async def html_compare(self, request): else: cost_yesterday_no_car = cost_yesterday_hist - compare_list = self.get_arg("compare_list", []) + # Same entries, with the same string ids, as the comparison itself ran on (#5472) + compare_list = valid_compare_list(self.get_arg("compare_list", [])) compare_hist = {} for item in compare_list: id = item.get("id", None) @@ -4465,8 +4462,6 @@ async def html_compare(self, request): compare_hist[id]["metric"] = history_attribute(entity_hist, state_key="metric", attributes=True, daily=True, pounds=True) compare_hist[id]["cost_1am"] = self.history_daily_at_hour(entity_hist) - compare_list = self.get_arg("compare_list", []) - for compare in compare_list: name = compare.get("name", "") id = compare.get("id", "") From 318b15b5faf1912a9aefee58e861d41f0ed4d16c Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:08:57 +0100 Subject: [PATCH 5/9] fix(compare): one check_compare_list() for engine, editor and page (#5472) Review follow-up. check_compare_list() returns the usable entries plus a message per skipped one, so web.py's validator no longer re-implements the loop. It also covers a compare_list that isn't a list (was a TypeError every cycle via publish_only), strips padded ids so ' agile' can't sidestep the duplicate check, and str()s a numeric name, which otherwise crashed publish_data() - and, once saved, every startup. The Compare page now says why an entry was skipped instead of dropping it silently or claiming nothing is configured. Tests now assert the run_all() warnings and the editor's refusal reasons. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/compare.py | 58 ++++++++++++++---------- apps/predbat/tests/test_compare.py | 24 +++++++--- apps/predbat/tests/test_web_apps_edit.py | 8 ++-- apps/predbat/tests/test_web_functions.py | 21 +++++++-- apps/predbat/web.py | 24 +++++----- 5 files changed, 87 insertions(+), 48 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index 9c2e9842a..c292e640a 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -27,36 +27,46 @@ # 3. Consider Octopus API key to access current tariff info and switch links -def compare_entry_problem(entry, seen_ids): - """Return why a compare_list entry can't be compared, or None if it can (#5472).""" - if not isinstance(entry, dict): - return "must be a dictionary with an id and a name" - raw_id = entry.get("id") - entry_id = "" if raw_id is None else str(raw_id) - if not entry_id.strip(): - return "requires a non-empty 'id'" - if entry_id in seen_ids: - return "has a duplicate id '{}'".format(entry_id) - if not entry.get("name"): - return "('{}') requires a non-empty 'name'".format(entry_id) - return None +def check_compare_list(compare_list): + """Split compare_list into the entries that can be compared (ids and names as strings) and a message per skipped entry (#5472).""" + if not compare_list: + return [], [] + if not isinstance(compare_list, list): + return [], ["compare_list must be a list of tariffs, each with an id and a name"] - -def valid_compare_list(compare_list, log=None): - """Return the compare_list entries that can be compared, with ids as strings, logging each one skipped (#5472).""" valid = [] + problems = [] seen_ids = set() - for index, entry in enumerate(compare_list or []): - problem = compare_entry_problem(entry, seen_ids) + for index, entry in enumerate(compare_list): + problem = None + entry_id = "" + if not isinstance(entry, dict): + problem = "must be a dictionary with an id and a name" + else: + # Results, sensor names and the web page are all keyed by the id as a string, so an + # unquoted numeric id in apps.yaml must not reach them as an int + entry_id = "" if entry.get("id") is None else str(entry["id"]).strip() + if not entry_id: + problem = "requires a non-empty 'id'" + elif entry_id in seen_ids: + problem = "has a duplicate id '{}'".format(entry_id) + elif not entry.get("name"): + problem = "('{}') requires a non-empty 'name'".format(entry_id) if problem: - if log: - log("Warn: Compare, compare_list entry {} {}, skipping it".format(index + 1, problem)) + problems.append("compare_list entry {} {}".format(index + 1, problem)) continue - # Results, sensor names and the web page are all keyed by the id as a string, so an - # unquoted numeric id in apps.yaml must not reach them as an int - entry_id = str(entry["id"]) seen_ids.add(entry_id) - valid.append(dict(entry, id=entry_id)) + # A numeric name would likewise break publish_data() building the friendly name + valid.append(dict(entry, id=entry_id, name=str(entry["name"]))) + return valid, problems + + +def valid_compare_list(compare_list, log=None): + """Return the compare_list entries that can be compared, logging a warning for each one skipped (#5472).""" + valid, problems = check_compare_list(compare_list) + if log: + for problem in problems: + log("Warn: Compare, {}, skipping it".format(problem)) return valid diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index f8e906057..61a7663e5 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -23,7 +23,7 @@ import tempfile from unittest.mock import MagicMock, patch -from compare import Compare, valid_compare_list +from compare import Compare, check_compare_list, valid_compare_list from const import MINUTE_WATT @@ -737,16 +737,21 @@ def _mock_plan_iboost_smart(): {"id": "agile", "name": "Agile Again"}, {"id": "no_name"}, {"id": 2024, "name": "Numeric Id Tariff"}, + {"id": " agile ", "name": "Padded Agile"}, + {"id": "numeric_name", "name": 1234}, ] kept = valid_compare_list(entries, log=logged.append) kept_ids = [entry["id"] for entry in kept] skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) - if kept_ids != ["agile", "2024"]: - print("ERROR T27: valid_compare_list() should keep only 'agile' and '2024' (as a string), got {}".format(kept_ids)) + if kept_ids != ["agile", "2024", "numeric_name"]: + print("ERROR T27: valid_compare_list() should keep only 'agile', '2024' and 'numeric_name' (ids as strings), got {}".format(kept_ids)) failed += 1 - elif skipped_positions != [1, 2, 3, 5, 6]: - print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5 and 6, got {}".format(logged)) + elif skipped_positions != [1, 2, 3, 5, 6, 8]: + print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6 and 8 (padded duplicate id), got {}".format(logged)) + failed += 1 + elif kept[2]["name"] != "1234": + print("ERROR T27: a numeric name should become a string, or publish_data() can't build the friendly name, got {!r}".format(kept[2]["name"])) failed += 1 elif kept[0] is entries[3] or entries[6]["id"] != 2024: print("ERROR T27: valid_compare_list() must return copies, not rewrite the user's compare_list in place") @@ -754,6 +759,9 @@ def _mock_plan_iboost_smart(): elif valid_compare_list(None) != [] or valid_compare_list([{"id": 0, "name": "Zero"}]) != [{"id": "0", "name": "Zero"}]: print("ERROR T27: valid_compare_list() should accept a missing list and an id of 0") failed += 1 + elif check_compare_list("agile") != ([], ["compare_list must be a list of tariffs, each with an id and a name"]) or check_compare_list(5)[0] != []: + print("ERROR T27: a compare_list that isn't a list should give no entries and one problem, got {} / {}".format(check_compare_list("agile"), check_compare_list(5))) + failed += 1 else: print("PASS T27: valid_compare_list() skips malformed entries with a positioned warning and normalises ids to strings") @@ -764,7 +772,8 @@ def _mock_plan_iboost_smart(): # ------------------------------------------------------------------ cmp = Compare.__new__(Compare) cmp.pb = my_predbat - cmp.log = lambda msg: None + run_all_logged = [] + cmp.log = run_all_logged.append cmp.comparisons = {} cmp.live_saving_state = None run_single_ids = [] @@ -798,6 +807,9 @@ def fake_get_arg(key, default=None, *args, **kwargs): elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid", "2024"}: print("ERROR T28: only the valid tariffs' results should be persisted, keyed by string id, got {}".format(saved_comparisons)) failed += 1 + elif sorted(msg.split("compare_list entry ")[1][0] for msg in run_all_logged if "skipping it" in msg) != ["1", "2", "4"]: + print("ERROR T28: run_all() should log a warning for each skipped entry (1, 2 and 4), got {}".format(run_all_logged)) + failed += 1 else: print("PASS T28: run_all() skips malformed entries and compares and persists the valid ones by string id") diff --git a/apps/predbat/tests/test_web_apps_edit.py b/apps/predbat/tests/test_web_apps_edit.py index c9864a3a0..7f84e8309 100644 --- a/apps/predbat/tests/test_web_apps_edit.py +++ b/apps/predbat/tests/test_web_apps_edit.py @@ -384,16 +384,16 @@ def run_web_apps_edit_tests(my_predbat): print("Test: a compare profile with a blank id is refused (#5472)") web_interface = _reset_fixture(my_predbat) result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", 'name: Blank\nid: " "\n')}) - if result.get("success"): - print(" ERROR: expected the blank id to be refused, got: {}".format(result)) + if result.get("success") or "requires a non-empty 'id'" not in result.get("message", ""): + print(" ERROR: expected the blank id to be refused for its id, got: {}".format(result)) failed += 1 # --------------------------------------------------------------------- print("Test: a numeric compare profile id clashing with the same id quoted is refused, since both publish one sensor (#5472)") web_interface = _reset_fixture(my_predbat) result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", "name: Numeric\nid: 7\n"), "compare_list[]#2": _add("compare_list[]", 'name: Quoted\nid: "7"\n')}) - if result.get("success"): - print(" ERROR: expected the numeric/quoted id clash to be refused, got: {}".format(result)) + if result.get("success") or "duplicate id '7'" not in result.get("message", ""): + print(" ERROR: expected the numeric/quoted id clash to be refused as a duplicate, got: {}".format(result)) failed += 1 # --------------------------------------------------------------------- diff --git a/apps/predbat/tests/test_web_functions.py b/apps/predbat/tests/test_web_functions.py index 0ba777033..f3553d40c 100644 --- a/apps/predbat/tests/test_web_functions.py +++ b/apps/predbat/tests/test_web_functions.py @@ -349,11 +349,26 @@ def run_compare_empty_state_tests(my_predbat, web): with patch.object(my_predbat, "comparison", fake_comparison): text = asyncio.run(web.html_compare(None)).text except Exception as exc: - text = "" + text = None print(f" ERROR: html_compare() raised {exc!r} on a compare_list with malformed entries") failed += 1 - if text and "NUMERIC_ID_PLAN" not in text: - print(f" ERROR: expected the plan stored under '2024' to be shown for the numeric id") + if text is not None: + if "NUMERIC_ID_PLAN" not in text: + print(f" ERROR: expected the plan stored under '2024' to be shown for the numeric id") + failed += 1 + if "compare_list entry 1 must be a dictionary" not in text or "compare_list entry 2 requires a non-empty 'id'" not in text: + print(f" ERROR: expected the page to say why entries 1 and 2 were skipped") + failed += 1 + + # ------------------------------------------------------------------------- + print("Test: a compare_list with only malformed entries explains them instead of 'not configured' or a stuck loading message (#5472)") + my_predbat.args["compare_list"] = [{"name": "No Id"}] + text = asyncio.run(web.html_compare(None)).text + if "No tariffs configured yet" in text or "Loading chart (please wait)" in text: + print(f" ERROR: should not say 'not configured' or 'loading' when the configured entries are all malformed") + failed += 1 + if "compare_list entry 1 requires a non-empty" not in text: + print(f" ERROR: expected the page to say why the only entry was skipped") failed += 1 my_predbat.args = original_args diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 11efec799..0298d5988 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -96,7 +96,7 @@ from predbat import THIS_VERSION_DISPLAY from component_base import ComponentBase from config import APPS_SCHEMA -from compare import compare_entry_problem, valid_compare_list +from compare import check_compare_list import debug_history from web_annual import AnnualPage from web_chat import WebChat @@ -4088,12 +4088,9 @@ def _validate_compare_list(self, compare_list): if not compare_list: return - seen_ids = set() - for index, entry in enumerate(compare_list): - problem = compare_entry_problem(entry, seen_ids) - if problem: - raise ValueError("compare_list entry {} {}".format(index + 1, problem)) - seen_ids.add(str(entry["id"])) + _, problems = check_compare_list(compare_list) + if problems: + raise ValueError(problems[0]) async def html_apps_post(self, request): """ @@ -4449,7 +4446,8 @@ async def html_compare(self, request): cost_yesterday_no_car = cost_yesterday_hist # Same entries, with the same string ids, as the comparison itself ran on (#5472) - compare_list = valid_compare_list(self.get_arg("compare_list", [])) + configured_compare_list = self.get_arg("compare_list", []) + compare_list, compare_problems = check_compare_list(configured_compare_list) compare_hist = {} for item in compare_list: id = item.get("id", None) @@ -4521,6 +4519,10 @@ async def html_compare(self, request): text += "\n" + # The comparison only logs skipped entries when it runs, so say why a configured tariff is missing here + for problem in compare_problems: + text += "

Skipped: {} - fix it in apps.yaml

\n".format(html_module.escape(problem)) + # Charts text += '
' series_data = [] @@ -4536,9 +4538,9 @@ async def html_compare(self, request): if compare_hist: text += self.render_chart(series_data, self.currency_symbols[0], "Tariff Comparison - True cost", now_str, daily_chart=False) - elif not compare_list: + elif not configured_compare_list: text += '

No tariffs configured yet - see Comparing Energy Tariffs for how to add some to apps.yaml


' - else: + elif compare_list: text += "

Loading chart (please wait)...


" # 7-day rolling average chart @@ -4555,7 +4557,7 @@ async def html_compare(self, request): if series_7d: text += self.render_chart(series_7d, self.currency_symbols[0], "Tariff Comparison - 7 day rolling average", now_str, tagname="chart7d", daily_chart=False) elif not compare_list: - pass # Already explained by the "No tariffs configured" message above + pass # Already explained above, by "No tariffs configured" or the skipped entries else: text += "

7 day rolling average chart loading (please wait)...


" From 0872b6a22cca17486dda0c045b22449ae43f06ce Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:15:45 +0100 Subject: [PATCH 6/9] fix(compare): reject a non-mapping config, blank name; report every problem (#5472) Second review round. check_compare_list() now skips an entry whose config: isn't a mapping (it crashed run_all() outside the per-tariff try) and treats an empty config: as no overrides; strips the name so a blank one is refused like a blank id. publish_data() str()s a stored name so one saved as a number can't stop startup. The editor lists every compare_list problem at once rather than one per save. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/compare.py | 17 +++++++--- apps/predbat/tests/test_compare.py | 40 +++++++++++++++++++++--- apps/predbat/tests/test_web_apps_edit.py | 6 ++-- apps/predbat/web.py | 8 ++--- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index c292e640a..41ce67189 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -46,18 +46,26 @@ def check_compare_list(compare_list): # Results, sensor names and the web page are all keyed by the id as a string, so an # unquoted numeric id in apps.yaml must not reach them as an int entry_id = "" if entry.get("id") is None else str(entry["id"]).strip() + # A numeric name would likewise break publish_data() building the friendly name + entry_name = "" if entry.get("name") is None else str(entry["name"]).strip() if not entry_id: problem = "requires a non-empty 'id'" elif entry_id in seen_ids: problem = "has a duplicate id '{}'".format(entry_id) - elif not entry.get("name"): + elif not entry_name: problem = "('{}') requires a non-empty 'name'".format(entry_id) + elif entry.get("config") is not None and not isinstance(entry["config"], dict): + # fetch_config() looks each override up by key, outside run_all()'s per-tariff try + problem = "('{}') has a 'config' that isn't a set of 'setting: value' overrides".format(entry_id) if problem: problems.append("compare_list entry {} {}".format(index + 1, problem)) continue seen_ids.add(entry_id) - # A numeric name would likewise break publish_data() building the friendly name - valid.append(dict(entry, id=entry_id, name=str(entry["name"]))) + checked = dict(entry, id=entry_id, name=entry_name) + if checked.get("config") is None: + # An empty 'config:' line loads as None and means no overrides + checked.pop("config", None) + valid.append(checked) return valid, problems @@ -557,7 +565,8 @@ def publish_data(self): if result: cost = result.get("cost", 0) - name = result.get("name", "") + # str(): a numeric name saved before names were normalised (#5472) must not crash startup + name = str(result.get("name", "")) attributes = { "friendly_name": "Compare " + name, "state_class": "measurement", diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index 61a7663e5..9668a254a 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -739,16 +739,22 @@ def _mock_plan_iboost_smart(): {"id": 2024, "name": "Numeric Id Tariff"}, {"id": " agile ", "name": "Padded Agile"}, {"id": "numeric_name", "name": 1234}, + {"id": "blank_name", "name": " "}, + {"id": "bad_config", "name": "Bad Config", "config": ["metric_battery_cycle"]}, + {"id": "empty_config", "name": "Empty Config", "config": None}, ] kept = valid_compare_list(entries, log=logged.append) kept_ids = [entry["id"] for entry in kept] skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) - if kept_ids != ["agile", "2024", "numeric_name"]: - print("ERROR T27: valid_compare_list() should keep only 'agile', '2024' and 'numeric_name' (ids as strings), got {}".format(kept_ids)) + if kept_ids != ["agile", "2024", "numeric_name", "empty_config"]: + print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name' and 'empty_config' (ids as strings), got {}".format(kept_ids)) failed += 1 - elif skipped_positions != [1, 2, 3, 5, 6, 8]: - print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6 and 8 (padded duplicate id), got {}".format(logged)) + elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11]: + print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6, 8 (padded duplicate id), 10 (blank name) and 11 (non-mapping config), got {}".format(logged)) + failed += 1 + elif "config" in kept[3]: + print("ERROR T27: an empty 'config:' (None) should be dropped as no overrides, got {}".format(kept[3])) failed += 1 elif kept[2]["name"] != "1234": print("ERROR T27: a numeric name should become a string, or publish_data() can't build the friendly name, got {!r}".format(kept[2]["name"])) @@ -807,7 +813,7 @@ def fake_get_arg(key, default=None, *args, **kwargs): elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid", "2024"}: print("ERROR T28: only the valid tariffs' results should be persisted, keyed by string id, got {}".format(saved_comparisons)) failed += 1 - elif sorted(msg.split("compare_list entry ")[1][0] for msg in run_all_logged if "skipping it" in msg) != ["1", "2", "4"]: + elif sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in run_all_logged if "skipping it" in msg) != [1, 2, 4]: print("ERROR T28: run_all() should log a warning for each skipped entry (1, 2 and 4), got {}".format(run_all_logged)) failed += 1 else: @@ -883,6 +889,30 @@ def record_expose_config(name, value, *args, **kwargs): else: print("PASS T30: load_yaml() normalises a numeric result key to a string and publishes it") + # ------------------------------------------------------------------ + # T31: publish_data() copes with a numeric name saved to comparisons.yaml + # before names were normalised - otherwise "Compare " + name raised + # TypeError inside Compare.__init__ and Predbat failed to start (#5472) + # ------------------------------------------------------------------ + cmp, pb = _make_compare() + published = {} + cmp.dashboard_item = lambda entity_id, **kwargs: published.update({entity_id: kwargs.get("attributes", {})}) + cmp.comparisons = {"numeric_name": {"name": 2024, "cost": 1.0}} + try: + cmp.publish_data() + publish_error = None + except Exception as exc: + publish_error = exc + + if publish_error is not None: + print("ERROR T31: publish_data() raised {!r} on a saved numeric name".format(publish_error)) + failed += 1 + elif published.get("predbat.compare_tariff_numeric_name", {}).get("friendly_name") != "Compare 2024": + print("ERROR T31: expected friendly name 'Compare 2024', got {}".format(published)) + failed += 1 + else: + print("PASS T31: publish_data() publishes a saved numeric name as text") + if failed: print("**** compare tests FAILED: {} errors ****\n".format(failed)) else: diff --git a/apps/predbat/tests/test_web_apps_edit.py b/apps/predbat/tests/test_web_apps_edit.py index 7f84e8309..e867c86d2 100644 --- a/apps/predbat/tests/test_web_apps_edit.py +++ b/apps/predbat/tests/test_web_apps_edit.py @@ -383,9 +383,9 @@ def run_web_apps_edit_tests(my_predbat): # --------------------------------------------------------------------- print("Test: a compare profile with a blank id is refused (#5472)") web_interface = _reset_fixture(my_predbat) - result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", 'name: Blank\nid: " "\n')}) - if result.get("success") or "requires a non-empty 'id'" not in result.get("message", ""): - print(" ERROR: expected the blank id to be refused for its id, got: {}".format(result)) + result = _post_changes(web_interface, {"compare_list[]#1": _add("compare_list[]", 'name: Blank\nid: " "\n'), "compare_list[]#2": _add("compare_list[]", "name: Clash\nid: one\n")}) + if result.get("success") or "entry 4 requires a non-empty 'id'" not in result.get("message", "") or "entry 5 has a duplicate id 'one'" not in result.get("message", ""): + print(" ERROR: expected the blank id and the duplicate to be refused, naming both at once, got: {}".format(result)) failed += 1 # --------------------------------------------------------------------- diff --git a/apps/predbat/web.py b/apps/predbat/web.py index 0298d5988..d3ab5c45a 100644 --- a/apps/predbat/web.py +++ b/apps/predbat/web.py @@ -4083,14 +4083,12 @@ def _validate_compare_list(self, compare_list): """ Check every compare_list profile still has the unique id and non-empty name that compare.py indexes results by, raising ValueError if a batch has left one without. - Uses compare.py's own rules, so the editor refuses exactly what a comparison would skip. + Uses compare.py's own rules, so the editor refuses exactly what a comparison would skip, + and names every problem at once so they can all be fixed in one go. """ - if not compare_list: - return - _, problems = check_compare_list(compare_list) if problems: - raise ValueError(problems[0]) + raise ValueError("; ".join(problems)) async def html_apps_post(self, request): """ From 7c50c22c4b44715ddf0a542a3d36c089c2390a14 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:22:44 +0100 Subject: [PATCH 7/9] fix(compare): keep treating an empty config: list or string as no overrides (#5472) Third review round. The previous commit skipped any entry whose config: wasn't a mapping, which wrongly dropped working tariffs with config: [] or "" (both always meant no overrides). Only a non-empty non-mapping config - which crashed run_all()'s override snapshot - is skipped now, and the comment names that real crash site. T28 now drives a list config through run_all() itself. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/compare.py | 9 +++++---- apps/predbat/tests/test_compare.py | 19 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index 41ce67189..cb5e04756 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -54,16 +54,17 @@ def check_compare_list(compare_list): problem = "has a duplicate id '{}'".format(entry_id) elif not entry_name: problem = "('{}') requires a non-empty 'name'".format(entry_id) - elif entry.get("config") is not None and not isinstance(entry["config"], dict): - # fetch_config() looks each override up by key, outside run_all()'s per-tariff try + elif entry.get("config") and not isinstance(entry["config"], dict): + # run_all() snapshots each override by key before any tariff runs, outside every try, + # so a list of overrides there crashed the whole comparison problem = "('{}') has a 'config' that isn't a set of 'setting: value' overrides".format(entry_id) if problem: problems.append("compare_list entry {} {}".format(index + 1, problem)) continue seen_ids.add(entry_id) checked = dict(entry, id=entry_id, name=entry_name) - if checked.get("config") is None: - # An empty 'config:' line loads as None and means no overrides + if not checked.get("config"): + # An empty 'config:' line (None, [] or "") means no overrides, as it always has checked.pop("config", None) valid.append(checked) return valid, problems diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index 9668a254a..c5db8b3f3 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -742,19 +742,23 @@ def _mock_plan_iboost_smart(): {"id": "blank_name", "name": " "}, {"id": "bad_config", "name": "Bad Config", "config": ["metric_battery_cycle"]}, {"id": "empty_config", "name": "Empty Config", "config": None}, + {"id": "empty_list_config", "name": " Empty List Config ", "config": []}, ] kept = valid_compare_list(entries, log=logged.append) kept_ids = [entry["id"] for entry in kept] skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) - if kept_ids != ["agile", "2024", "numeric_name", "empty_config"]: - print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name' and 'empty_config' (ids as strings), got {}".format(kept_ids)) + if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config"]: + print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name', 'empty_config' and 'empty_list_config' (ids as strings), got {}".format(kept_ids)) failed += 1 elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11]: print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6, 8 (padded duplicate id), 10 (blank name) and 11 (non-mapping config), got {}".format(logged)) failed += 1 - elif "config" in kept[3]: - print("ERROR T27: an empty 'config:' (None) should be dropped as no overrides, got {}".format(kept[3])) + elif "config" in kept[3] or "config" in kept[4]: + print("ERROR T27: an empty 'config:' (None or []) should be dropped as no overrides, got {} / {}".format(kept[3], kept[4])) + failed += 1 + elif kept[4]["name"] != "Empty List Config": + print("ERROR T27: a kept entry's name should be stripped, got {!r}".format(kept[4]["name"])) failed += 1 elif kept[2]["name"] != "1234": print("ERROR T27: a numeric name should become a string, or publish_data() can't build the friendly name, got {!r}".format(kept[2]["name"])) @@ -788,7 +792,8 @@ def _mock_plan_iboost_smart(): cmp.select_best = lambda compare_list, results: None cmp.save_yaml = lambda: saved_comparisons.append(dict(cmp.comparisons)) cmp.publish_data = lambda: None - compare_list = [{"name": "No Id Tariff"}, "not_a_dict", {"id": "valid", "name": "Valid Tariff"}, {"id": "valid", "name": "Duplicate Tariff"}, {"id": 2024, "name": "Numeric Id Tariff"}] + # The config: entries go through run_all()'s own override snapshot, which runs before any try + compare_list = [{"name": "No Id Tariff"}, "not_a_dict", {"id": "valid", "name": "Valid Tariff", "config": None}, {"id": "valid", "name": "Duplicate Tariff"}, {"id": 2024, "name": "Numeric Id Tariff"}, {"id": "list_config", "name": "List Config", "config": [{"metric_battery_cycle": 1}]}] real_get_arg = my_predbat.get_arg def fake_get_arg(key, default=None, *args, **kwargs): @@ -813,8 +818,8 @@ def fake_get_arg(key, default=None, *args, **kwargs): elif not saved_comparisons or set(saved_comparisons[-1].keys()) != {"valid", "2024"}: print("ERROR T28: only the valid tariffs' results should be persisted, keyed by string id, got {}".format(saved_comparisons)) failed += 1 - elif sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in run_all_logged if "skipping it" in msg) != [1, 2, 4]: - print("ERROR T28: run_all() should log a warning for each skipped entry (1, 2 and 4), got {}".format(run_all_logged)) + elif sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in run_all_logged if "skipping it" in msg) != [1, 2, 4, 6]: + print("ERROR T28: run_all() should log a warning for each skipped entry (1, 2, 4 and 6), got {}".format(run_all_logged)) failed += 1 else: print("PASS T28: run_all() skips malformed entries and compares and persists the valid ones by string id") From f41a277a7cc978ec3396ea8b17bd0db1db32ac14 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:27:52 +0100 Subject: [PATCH 8/9] fix(compare): spell out which config: values mean no overrides (#5472) Review follow-up. Only None, "", [] and {} count as an empty config: and are dropped; any other non-mapping (false, 0, a string, a list) is skipped with a warning instead of silently losing the overrides. check_compare_list()'s docstring now states the shape it guarantees, and the comment names where a list config really crashed (overrides looked up by key in run_all()/fetch_config()). Co-Authored-By: Claude Opus 5.5 --- apps/predbat/compare.py | 14 +++++++++----- apps/predbat/tests/test_compare.py | 16 ++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index cb5e04756..f02b9bd65 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -28,7 +28,11 @@ def check_compare_list(compare_list): - """Split compare_list into the entries that can be compared (ids and names as strings) and a message per skipped entry (#5472).""" + """Split compare_list into the entries that can be compared and a message per skipped entry (#5472). + + Each kept entry is a copy with its id and name as stripped strings, and its config either a + mapping of overrides or absent, so run_all() and fetch_config() can rely on that shape. + """ if not compare_list: return [], [] if not isinstance(compare_list, list): @@ -54,9 +58,9 @@ def check_compare_list(compare_list): problem = "has a duplicate id '{}'".format(entry_id) elif not entry_name: problem = "('{}') requires a non-empty 'name'".format(entry_id) - elif entry.get("config") and not isinstance(entry["config"], dict): - # run_all() snapshots each override by key before any tariff runs, outside every try, - # so a list of overrides there crashed the whole comparison + elif entry.get("config") not in (None, "", []) and not isinstance(entry["config"], dict): + # run_all() and fetch_config() look overrides up by key; a list of them raised TypeError + # there and ended the whole comparison, and any other value was never a usable override problem = "('{}') has a 'config' that isn't a set of 'setting: value' overrides".format(entry_id) if problem: problems.append("compare_list entry {} {}".format(index + 1, problem)) @@ -64,7 +68,7 @@ def check_compare_list(compare_list): seen_ids.add(entry_id) checked = dict(entry, id=entry_id, name=entry_name) if not checked.get("config"): - # An empty 'config:' line (None, [] or "") means no overrides, as it always has + # An empty 'config:' line (None, "", [] or {}) means no overrides, as it always has checked.pop("config", None) valid.append(checked) return valid, problems diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index c5db8b3f3..4daca7f9e 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -743,19 +743,23 @@ def _mock_plan_iboost_smart(): {"id": "bad_config", "name": "Bad Config", "config": ["metric_battery_cycle"]}, {"id": "empty_config", "name": "Empty Config", "config": None}, {"id": "empty_list_config", "name": " Empty List Config ", "config": []}, + {"id": "empty_string_config", "name": "Empty String Config", "config": ""}, + {"id": "empty_dict_config", "name": "Empty Dict Config", "config": {}}, + {"id": "false_config", "name": "False Config", "config": False}, + {"id": "string_config", "name": "String Config", "config": "best_soc_min: 2.0"}, ] kept = valid_compare_list(entries, log=logged.append) kept_ids = [entry["id"] for entry in kept] skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) - if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config"]: - print("ERROR T27: valid_compare_list() should keep only 'agile', '2024', 'numeric_name', 'empty_config' and 'empty_list_config' (ids as strings), got {}".format(kept_ids)) + if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config"]: + 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)) failed += 1 - elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11]: - print("ERROR T27: valid_compare_list() should warn for entries 1, 2, 3, 5, 6, 8 (padded duplicate id), 10 (blank name) and 11 (non-mapping config), got {}".format(logged)) + elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11, 16, 17]: + 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)) failed += 1 - elif "config" in kept[3] or "config" in kept[4]: - print("ERROR T27: an empty 'config:' (None or []) should be dropped as no overrides, got {} / {}".format(kept[3], kept[4])) + elif any("config" in entry for entry in kept[3:7]): + print("ERROR T27: an empty 'config:' (None, [], \"\" or {{}}) should be dropped as no overrides, got {}".format(kept[3:7])) failed += 1 elif kept[4]["name"] != "Empty List Config": print("ERROR T27: a kept entry's name should be stripped, got {!r}".format(kept[4]["name"])) From b7f37dcc4b27aaa417973cdd22caed9e3564661f Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sun, 11 Oct 2026 23:34:30 +0100 Subject: [PATCH 9/9] 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 --- apps/predbat/compare.py | 9 ++++++--- apps/predbat/tests/test_compare.py | 20 ++++++++++++++------ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/predbat/compare.py b/apps/predbat/compare.py index f02b9bd65..d9c242aa7 100644 --- a/apps/predbat/compare.py +++ b/apps/predbat/compare.py @@ -27,6 +27,10 @@ # 3. Consider Octopus API key to access current tariff info and switch links +# An empty 'config:' line means no overrides; any other non-mapping value can't be applied +EMPTY_COMPARE_CONFIG = (None, "", [], {}) + + def check_compare_list(compare_list): """Split compare_list into the entries that can be compared and a message per skipped entry (#5472). @@ -58,7 +62,7 @@ def check_compare_list(compare_list): problem = "has a duplicate id '{}'".format(entry_id) elif not entry_name: problem = "('{}') requires a non-empty 'name'".format(entry_id) - elif entry.get("config") not in (None, "", []) and not isinstance(entry["config"], dict): + elif entry.get("config") not in EMPTY_COMPARE_CONFIG and not isinstance(entry["config"], dict): # run_all() and fetch_config() look overrides up by key; a list of them raised TypeError # there and ended the whole comparison, and any other value was never a usable override 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): continue seen_ids.add(entry_id) checked = dict(entry, id=entry_id, name=entry_name) - if not checked.get("config"): - # An empty 'config:' line (None, "", [] or {}) means no overrides, as it always has + if checked.get("config") in EMPTY_COMPARE_CONFIG: checked.pop("config", None) valid.append(checked) return valid, problems diff --git a/apps/predbat/tests/test_compare.py b/apps/predbat/tests/test_compare.py index 4daca7f9e..99d8ca640 100644 --- a/apps/predbat/tests/test_compare.py +++ b/apps/predbat/tests/test_compare.py @@ -723,10 +723,11 @@ def _mock_plan_iboost_smart(): # ------------------------------------------------------------------ # T27: valid_compare_list() keeps only the entries run_all() can compare - # skipping, with a warning naming the entry's position, one that isn't a - # dict, has no id, repeats an earlier id or has no name - and turns a - # numeric id into a string (#5472 - run_all() used to index tariff["id"] - # directly, raising KeyError every cycle, and a numeric id still crashed - # publish_data() building the sensor name) + # dict, has no id, repeats an earlier id, has no name or has a config: + # that is neither a mapping nor empty (None, "", [] and {} mean no + # overrides) - and turns a numeric id into a string (#5472 - run_all() + # used to index tariff["id"] directly, raising KeyError every cycle, and a + # numeric id still crashed publish_data() building the sensor name) # ------------------------------------------------------------------ logged = [] entries = [ @@ -747,17 +748,24 @@ def _mock_plan_iboost_smart(): {"id": "empty_dict_config", "name": "Empty Dict Config", "config": {}}, {"id": "false_config", "name": "False Config", "config": False}, {"id": "string_config", "name": "String Config", "config": "best_soc_min: 2.0"}, + {"id": "mapping_config", "name": "Mapping Config", "config": {"best_soc_min": 2.0}}, ] kept = valid_compare_list(entries, log=logged.append) kept_ids = [entry["id"] for entry in kept] skipped_positions = sorted(int(msg.split("compare_list entry ")[1].split(" ")[0]) for msg in logged) - if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config"]: - 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)) + if kept_ids != ["agile", "2024", "numeric_name", "empty_config", "empty_list_config", "empty_string_config", "empty_dict_config", "mapping_config"]: + 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)) failed += 1 elif skipped_positions != [1, 2, 3, 5, 6, 8, 10, 11, 16, 17]: 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)) failed += 1 + elif not all("'config'" in msg for msg in logged if msg.split("compare_list entry ")[1].split(" ")[0] in ("11", "16", "17")): + print("ERROR T27: entries 11, 16 and 17 should be skipped for their config, got {}".format(logged)) + failed += 1 + elif kept[7].get("config") != {"best_soc_min": 2.0}: + print("ERROR T27: a mapping config: should be kept as the tariff's overrides, got {}".format(kept[7])) + failed += 1 elif any("config" in entry for entry in kept[3:7]): print("ERROR T27: an empty 'config:' (None, [], \"\" or {{}}) should be dropped as no overrides, got {}".format(kept[3:7])) failed += 1