From 79132d07a76de153949aed1eb0056d60bbb67252 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Fri, 31 Jul 2026 12:12:30 +0200 Subject: [PATCH 1/2] test(drivers): let a driver report metrics, and a topic carry a configurable prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two catalog conventions only held because every driver that breaks them is byte-identical to its FTW baseline and therefore skipped. Editing one for any reason fails on a rule it never had to meet, and neither rule was right. test_calls_emit_in_poll required host.emit, which carries a DER reading and needs a DER type the host understands. VALID_DERS has had `heatpump` since the FTW promotion, but no host has a heat-pump reading type, so a heat-pump driver has nothing to emit and reports through host.emit_metric. The rule left an author two options: emit nothing, or claim to be a battery. test_subscription_topics_are_strings required a literal, while heishamon and ctek_hybrid both build the topic from a file-local set in driver_init — the broker prefix is the operator's to choose. The check now asks for what it actually wants: a topic decidable without running the driver. Literals and file-local names concatenate; an index, a field or a call does not, which is how a driver ends up subscribing to whatever a message told it to. Co-Authored-By: Claude Opus 5 Signed-off-by: Fredrik Ahlgren --- drivers/tests/test_driver_contract.py | 16 ++++++++++++--- drivers/tests/test_mqtt_drivers.py | 28 ++++++++++++++++++++------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/drivers/tests/test_driver_contract.py b/drivers/tests/test_driver_contract.py index 5e42ff6..5c00fcb 100644 --- a/drivers/tests/test_driver_contract.py +++ b/drivers/tests/test_driver_contract.py @@ -72,12 +72,22 @@ def test_calls_set_make_in_init(self, driver_name): f"{driver_name}: should call host.set_make() in driver_init" def test_calls_emit_in_poll(self, driver_name): - """driver_poll should call host.emit().""" + """driver_poll should report something: a DER reading or metrics. + + `host.emit` carries a DER reading and needs a DER type the host + understands. `heatpump` is in VALID_DERS, but no host has a heat-pump + reading type yet, so a heat-pump driver has nothing to emit and + reports temperatures and power through `host.emit_metric` instead. + Requiring `host.emit` of it would leave the author two options: emit + nothing, or claim to be a battery. Both are worse than a driver that + says what it measures. + """ if driver_name == "hello": pytest.skip("hello driver is a demo-only driver") code = read_driver(driver_name) - assert 'host.emit(' in code, \ - f"{driver_name}: should call host.emit() in driver_poll" + assert 'host.emit(' in code or 'host.emit_metric(' in code, \ + f"{driver_name}: should call host.emit() or host.emit_metric() " \ + f"in driver_poll" def test_no_forbidden_globals(self, driver_name): """Driver must not use forbidden sandbox-escaping functions.""" diff --git a/drivers/tests/test_mqtt_drivers.py b/drivers/tests/test_mqtt_drivers.py index c9e78e0..167dfda 100644 --- a/drivers/tests/test_mqtt_drivers.py +++ b/drivers/tests/test_mqtt_drivers.py @@ -37,22 +37,36 @@ def test_subscribes_in_driver_init(self, driver_name): f"not elsewhere" ) - def test_subscription_topics_are_strings(self, driver_name): - """All mqtt_subscribe calls should use string literal topics.""" + def test_subscription_topics_are_static(self, driver_name): + """A subscription topic must be decidable without running the driver. + + A literal is the common case. A configurable prefix is the other one: + heishamon and ctek_hybrid both build the topic from a file-local set + in driver_init, because the broker prefix is the operator's to choose. + That stays reviewable — every part is in the file. + + What this rejects is a topic taken from runtime data: an index, a + field or a call, which is how a driver ends up subscribing to whatever + a message told it to. + """ code = read_driver(driver_name) clean = strip_lua_comments(code) - # Find all subscribe calls and verify they use string literals sub_calls = re.findall( r'host\.mqtt_subscribe\s*\(\s*(.+?)\s*\)', clean, ) for call_arg in sub_calls: - # Should be a string literal (starts with ") - assert call_arg.strip().startswith('"'), ( - f"{driver_name}: mqtt_subscribe should use string literal " - f"topic, found: {call_arg}" + arg = call_arg.strip() + static = re.fullmatch( + r'(?:"[^"]*"|[A-Za-z_]\w*)' + r'(?:\s*\.\.\s*(?:"[^"]*"|[A-Za-z_]\w*))*', + arg, + ) + assert static, ( + f"{driver_name}: mqtt_subscribe topic must be a literal or a " + f"concatenation of literals and file-local names, found: {arg}" ) From 6098eae19006396fd400a7e996893d7db35b0433 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Fri, 7 Aug 2026 09:59:22 +0200 Subject: [PATCH 2/2] test(drivers): tighten metric and MQTT exceptions Signed-off-by: Fredrik Ahlgren --- drivers/tests/test_driver_contract.py | 37 +++++++++++----- drivers/tests/test_mqtt_drivers.py | 62 ++++++++++++++++++++++----- 2 files changed, 78 insertions(+), 21 deletions(-) diff --git a/drivers/tests/test_driver_contract.py b/drivers/tests/test_driver_contract.py index 5c00fcb..d00e513 100644 --- a/drivers/tests/test_driver_contract.py +++ b/drivers/tests/test_driver_contract.py @@ -6,7 +6,12 @@ import re import pytest -from conftest import read_driver, get_driver_names, strip_lua_comments +from conftest import ( + read_driver, + read_manifest, + get_driver_names, + strip_lua_comments, +) DRIVERS = get_driver_names() @@ -72,22 +77,32 @@ def test_calls_set_make_in_init(self, driver_name): f"{driver_name}: should call host.set_make() in driver_init" def test_calls_emit_in_poll(self, driver_name): - """driver_poll should report something: a DER reading or metrics. + """driver_poll should report each declared DER through host.emit. `host.emit` carries a DER reading and needs a DER type the host - understands. `heatpump` is in VALID_DERS, but no host has a heat-pump - reading type yet, so a heat-pump driver has nothing to emit and - reports temperatures and power through `host.emit_metric` instead. - Requiring `host.emit` of it would leave the author two options: emit - nothing, or claim to be a battery. Both are worse than a driver that - says what it measures. + understands. Heat-pump and schema-less drivers have no matching + reading type, so they may report only through `host.emit_metric`. + Metrics remain diagnostic data for every schema-backed DER. """ if driver_name == "hello": pytest.skip("hello driver is a demo-only driver") code = read_driver(driver_name) - assert 'host.emit(' in code or 'host.emit_metric(' in code, \ - f"{driver_name}: should call host.emit() or host.emit_metric() " \ - f"in driver_poll" + if 'host.emit(' in code: + return + + manifest = read_manifest(driver_name) + match = re.search(r"^ders:\s*\[([^]]*)\]\s*$", manifest, re.MULTILINE) + assert match, f"{driver_name}: cannot parse manifest ders" + ders = { + item.strip().strip("'\"") + for item in match.group(1).split(",") + if item.strip() + } + metric_only = not ders or ders == {"heatpump"} + assert metric_only and 'host.emit_metric(' in code, ( + f"{driver_name}: declares {sorted(ders)} and must call host.emit(); " + f"only heat-pump or schema-less drivers may report metrics only" + ) def test_no_forbidden_globals(self, driver_name): """Driver must not use forbidden sandbox-escaping functions.""" diff --git a/drivers/tests/test_mqtt_drivers.py b/drivers/tests/test_mqtt_drivers.py index 167dfda..59cd45d 100644 --- a/drivers/tests/test_mqtt_drivers.py +++ b/drivers/tests/test_mqtt_drivers.py @@ -14,6 +14,51 @@ MQTT_DRIVERS = get_mqtt_drivers() +_TOPIC_PART = r'(?:(?:"(?:\\.|[^"\\])*")|(?:[A-Za-z_]\w*))' +_TOPIC_EXPRESSION = re.compile( + rf'{_TOPIC_PART}(?:\s*\.\.\s*{_TOPIC_PART})*' +) + + +def _file_local_names(code): + """Return names declared before the first function in a Lua file.""" + first_function = re.search( + r'^\s*(?:local\s+)?function\b', + code, + re.MULTILINE, + ) + preamble = code[:first_function.start()] if first_function else code + return set(re.findall( + r'^\s*local\s+([A-Za-z_]\w*)\s*=', + preamble, + re.MULTILINE, + )) + + +def _is_static_topic_expression(arg, file_local_names): + """Accept literals and concatenations of declared file-local names.""" + if not _TOPIC_EXPRESSION.fullmatch(arg): + return False + without_strings = re.sub(r'"(?:\\.|[^"\\])*"', '', arg) + names = set(re.findall(r'\b[A-Za-z_]\w*\b', without_strings)) + return names <= file_local_names + + +@pytest.mark.parametrize( + ("arg", "file_local_names", "expected"), + [ + ('"fixed/#"', set(), True), + ('BASE_TOPIC .. "/#"', {"BASE_TOPIC"}, True), + ('topic', set(), False), + ('BASE_TOPIC .. topic', {"BASE_TOPIC"}, False), + ('msg.topic', set(), False), + ('make_topic()', {"make_topic"}, False), + ], +) +def test_static_topic_expression_requires_file_local_names( + arg, file_local_names, expected): + assert _is_static_topic_expression(arg, file_local_names) is expected + @pytest.mark.parametrize("driver_name", MQTT_DRIVERS) class TestMqttSubscription: @@ -40,10 +85,9 @@ def test_subscribes_in_driver_init(self, driver_name): def test_subscription_topics_are_static(self, driver_name): """A subscription topic must be decidable without running the driver. - A literal is the common case. A configurable prefix is the other one: - heishamon and ctek_hybrid both build the topic from a file-local set - in driver_init, because the broker prefix is the operator's to choose. - That stays reviewable — every part is in the file. + A literal is the common case. A file-local constant or configurable + prefix is the other one. Function-local names do not qualify because + they can come from a received message. What this rejects is a topic taken from runtime data: an index, a field or a call, which is how a driver ends up subscribing to whatever @@ -51,6 +95,7 @@ def test_subscription_topics_are_static(self, driver_name): """ code = read_driver(driver_name) clean = strip_lua_comments(code) + file_names = _file_local_names(clean) sub_calls = re.findall( r'host\.mqtt_subscribe\s*\(\s*(.+?)\s*\)', @@ -59,14 +104,11 @@ def test_subscription_topics_are_static(self, driver_name): for call_arg in sub_calls: arg = call_arg.strip() - static = re.fullmatch( - r'(?:"[^"]*"|[A-Za-z_]\w*)' - r'(?:\s*\.\.\s*(?:"[^"]*"|[A-Za-z_]\w*))*', - arg, - ) + static = _is_static_topic_expression(arg, file_names) assert static, ( f"{driver_name}: mqtt_subscribe topic must be a literal or a " - f"concatenation of literals and file-local names, found: {arg}" + f"concatenation of literals and declared file-local names, " + f"found: {arg}" )