From 12b7f9f422584f2e97e84c9bd6aa4137f19ab850 Mon Sep 17 00:00:00 2001
From: grabowski <berwn@buildfor.life>
Date: Wed, 22 Jul 2026 14:02:06 +0700
Subject: [PATCH] Add assert-based pytest coverage for recent fixes

- tests/conftest.py: put repo root on sys.path so `import src...` resolves
  under pytest regardless of invocation directory.
- test_matrix_formatting.py: lock in HTML formatted_body + plain-text fallback,
  URL linkification, HTML escaping, and send_alert field rendering.
- test_station_persistence.py: cover default-load, save/reload round-trip
  (incl. Thai text), runtime-file precedence, and atomic-write cleanup.

These are real assert-based tests (unlike the existing print-style scripts) so
CI can gate on them. 13 tests, all passing.
---
 tests/conftest.py                 | 12 ++++
 tests/test_matrix_formatting.py   | 98 +++++++++++++++++++++++++++++++
 tests/test_station_persistence.py | 77 ++++++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 tests/conftest.py
 create mode 100644 tests/test_matrix_formatting.py
 create mode 100644 tests/test_station_persistence.py

diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 0000000..72521e0
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,12 @@
+"""Shared pytest configuration.
+
+Ensures the repository root is on sys.path so tests can import the ``src``
+package regardless of the working directory pytest is invoked from.
+"""
+
+import os
+import sys
+
+REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+if REPO_ROOT not in sys.path:
+    sys.path.insert(0, REPO_ROOT)
diff --git a/tests/test_matrix_formatting.py b/tests/test_matrix_formatting.py
new file mode 100644
index 0000000..ed1c082
--- /dev/null
+++ b/tests/test_matrix_formatting.py
@@ -0,0 +1,98 @@
+"""Assert-based tests for Matrix message formatting.
+
+Matrix clients only render formatting from an HTML ``formatted_body``; Markdown
+in the plain ``body`` shows as literal characters. These tests lock in that the
+notifier emits real HTML plus a clean plain-text fallback, and that untrusted
+station data is HTML-escaped.
+"""
+
+import datetime
+
+from src.alerting import AlertLevel, MatrixNotifier, WaterAlert, markdown_to_matrix_html, strip_markdown
+
+
+class _FakeResponse:
+    def raise_for_status(self):
+        pass
+
+    def json(self):
+        return {"event_id": "$test"}
+
+
+def _notifier_capturing(captured):
+    """A MatrixNotifier whose HTTP PUT records the JSON payload into ``captured``."""
+    notifier = MatrixNotifier("https://hs.example", "token", "!room:hs.example")
+
+    def fake_put(url, headers=None, json=None, timeout=None):
+        captured.update(json)
+        return _FakeResponse()
+
+    notifier.session.put = fake_put
+    return notifier
+
+
+def test_bold_becomes_strong():
+    assert markdown_to_matrix_html("**hi**") == "<strong>hi</strong>"
+
+
+def test_url_is_linkified():
+    out = markdown_to_matrix_html("see https://x.example/z")
+    assert '<a href="https://x.example/z">https://x.example/z</a>' in out
+
+
+def test_newlines_become_br():
+    assert markdown_to_matrix_html("a\nb") == "a<br/>b"
+
+
+def test_html_is_escaped():
+    out = markdown_to_matrix_html("<script> & 'stuff'")
+    assert "&lt;script&gt;" in out
+    assert "&amp;" in out
+    assert "<script>" not in out
+
+
+def test_strip_markdown_removes_bold_markers():
+    assert strip_markdown("**WATER LEVEL ALERT**") == "WATER LEVEL ALERT"
+
+
+def test_send_message_sends_html_and_plain_fallback():
+    captured = {}
+    notifier = _notifier_capturing(captured)
+
+    assert notifier.send_message("**hi** http://x.example") is True
+    assert captured["format"] == "org.matrix.custom.html"
+    assert "<strong>hi</strong>" in captured["formatted_body"]
+    # Plain body has the markdown markers stripped.
+    assert captured["body"] == "hi http://x.example"
+
+
+def test_send_message_plain_when_markdown_disabled():
+    captured = {}
+    notifier = _notifier_capturing(captured)
+
+    assert notifier.send_message("**raw**", markdown=False) is True
+    assert "formatted_body" not in captured
+    assert captured["body"] == "**raw**"
+
+
+def test_send_alert_renders_alert_fields():
+    captured = {}
+    notifier = _notifier_capturing(captured)
+    alert = WaterAlert(
+        station_code="P.1",
+        station_name="สะพานนวรัฐ",
+        alert_type="Zone 7 - Critical",
+        level=AlertLevel.CRITICAL,
+        water_level=4.62,
+        threshold=4.60,
+        discharge=612.0,
+        timestamp=datetime.datetime(2026, 7, 22, 14, 30, 0),
+    )
+
+    assert notifier.send_alert(alert) is True
+    html = captured["formatted_body"]
+    assert "<strong>WATER LEVEL ALERT</strong>" in html
+    assert "สะพานนวรัฐ" in html  # Thai station name preserved
+    assert "<strong>Current Level:</strong>" in html
+    # Plain fallback carries no leftover markdown markers.
+    assert "**" not in captured["body"]
diff --git a/tests/test_station_persistence.py b/tests/test_station_persistence.py
new file mode 100644
index 0000000..1439255
--- /dev/null
+++ b/tests/test_station_persistence.py
@@ -0,0 +1,77 @@
+"""Assert-based tests for station-mapping persistence.
+
+Station CRUD must survive restarts: the scraper loads its mapping from a
+runtime-writable JSON file (falling back to bundled defaults) and writes it back
+atomically. These tests exercise that load/save behaviour without constructing a
+full scraper (which would open network/DB connections).
+"""
+
+from src.water_scraper_v3 import EnhancedWaterMonitorScraper as Scraper
+
+
+def _bare_scraper(config_path):
+    """A scraper instance with only the station-config attribute set.
+
+    Bypasses __init__ so no database/HTTP connection is attempted.
+    """
+    scraper = Scraper.__new__(Scraper)
+    scraper.station_config_path = config_path
+    return scraper
+
+
+def test_loads_bundled_defaults_when_runtime_file_absent(tmp_path):
+    scraper = _bare_scraper(str(tmp_path / "does_not_exist.json"))
+    mapping = scraper._load_station_mapping()
+
+    assert len(mapping) == 16
+    assert mapping["8"]["code"] == "P.1"
+    assert mapping["8"]["english_name"] == "Nawarat Bridge"
+
+
+def test_save_then_reload_roundtrips_including_thai(tmp_path):
+    path = str(tmp_path / "stations.json")
+    scraper = _bare_scraper(path)
+    scraper.station_mapping = {
+        "1": {
+            "code": "P.99",
+            "thai_name": "สถานีทดสอบ",
+            "english_name": "Test Station",
+            "latitude": 1.0,
+            "longitude": 2.0,
+            "geohash": None,
+        }
+    }
+
+    assert scraper.save_stations() is True
+
+    reloaded = _bare_scraper(path)._load_station_mapping()
+    assert reloaded == scraper.station_mapping
+    assert reloaded["1"]["thai_name"] == "สถานีทดสอบ"
+
+
+def test_runtime_file_takes_precedence_over_defaults(tmp_path):
+    path = str(tmp_path / "stations.json")
+    writer = _bare_scraper(path)
+    writer.station_mapping = {"1": {"code": "ONLY"}}
+    assert writer.save_stations() is True
+
+    mapping = _bare_scraper(path)._load_station_mapping()
+    assert list(mapping.keys()) == ["1"]
+    assert mapping["1"]["code"] == "ONLY"
+
+
+def test_save_returns_false_without_a_path():
+    scraper = _bare_scraper("")
+    scraper.station_mapping = {}
+    assert scraper.save_stations() is False
+
+
+def test_save_is_atomic_no_tmp_left_behind(tmp_path):
+    path = tmp_path / "stations.json"
+    scraper = _bare_scraper(str(path))
+    scraper.station_mapping = {"1": {"code": "P.1"}}
+
+    assert scraper.save_stations() is True
+    assert path.exists()
+    # The temp file used during the atomic write must not remain.
+    assert not (tmp_path / "stations.json.tmp").exists()