From 3ad4ab22f4a1a75b0065a4d95a2bb7baa158955a Mon Sep 17 00:00:00 2001 From: Ryan Morash Date: Mon, 29 Jun 2026 16:39:40 -0400 Subject: [PATCH 1/2] chore(civicrm): remove unused _DEFAULT_ACTIVE_STATUSES constant CodeQL's quality suite flagged src/door_sync/civicrm/client.py:34 as an unused module variable. _DEFAULT_ACTIVE_STATUSES was defined but never referenced: the client reads self._config.active_statuses, and the config default ("Current","Grace","New") is defined independently in the config loader. Removing the stale duplicate eliminates a second, drift-prone "source of truth" for the default. No behavior change; pyrefly/ruff/pytest green (343 passed). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/door_sync/civicrm/client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/door_sync/civicrm/client.py b/src/door_sync/civicrm/client.py index 5128fbc..8f3ccd5 100644 --- a/src/door_sync/civicrm/client.py +++ b/src/door_sync/civicrm/client.py @@ -31,7 +31,6 @@ _CONTACT_BATCH_SIZE = ( 500 # Caps contact_ids per Membership.get IN clause to keep request body bounded ) -_DEFAULT_ACTIVE_STATUSES = ("Current", "Grace", "New") _MAX_PAGES = 1_000 # 250,000 records — far above any plausible deployment _MAX_ATTEMPTS = 3 From 30d64329425629f0cdece1febf13cd2cd7f47f4f Mon Sep 17 00:00:00 2001 From: Ryan Morash Date: Mon, 29 Jun 2026 16:45:49 -0400 Subject: [PATCH 2/2] test: address code-quality findings in test_alert / test_unifi_client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_alert.py: - Mailgun tests now assert resp.raise_for_status() is called (the HTTP status is checked, so a 4xx/5xx surfaces as a failure). Dropped the no-op lambda override that had blocked the assertion. - Dropped redundant manual __enter__/__exit__ on the SMTP server mock: mock_cls.return_value is a MagicMock that already supports the context manager (and __exit__ returns False, so it won't swallow exceptions); _send_smtp uses `with server:` with no `as`. test_unifi_client.py: - _patched_tls uses real ssl constants (CERT_NONE / PROTOCOL_TLS_CLIENT / TLSVersion) instead of magic 0s (PROTOCOL_TLS_CLIENT is 2, not 0). - Assert the FC-mismatch message with a plain "expected 42" string, not f"expected {42}". - Moved local imports (replace, config.load) to module top. - Added an explicit non-ASCII regular-digit case (Bengali ১) to the _parse_sync_alias rejection test, verifying the isascii() guard. 343 passed; pyrefly 0 errors; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_alert.py | 24 ++++++++++++++++-------- tests/test_unifi_client.py | 23 +++++++++++++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/tests/test_alert.py b/tests/test_alert.py index 13ac9a3..073cfe1 100644 --- a/tests/test_alert.py +++ b/tests/test_alert.py @@ -116,10 +116,12 @@ def test_raise_mailgun_sends_post(tmp_path: Path) -> None: cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) with patch("door_sync.alert.httpx.post") as mock_post: - mock_post.return_value.raise_for_status = lambda: None alert.raise_("safety halt", path=path, alert_config=cfg) mock_post.assert_called_once() + # The Mailgun response status must be checked (a 4xx/5xx should surface as a + # send failure, not a silent success). + mock_post.return_value.raise_for_status.assert_called_once() call_kwargs = mock_post.call_args # Assert the exact Mailgun endpoint, not a loose substring: the domain must # sit in the API path (https://api.mailgun.net/v3//messages), which @@ -137,10 +139,10 @@ def test_clear_mailgun_sends_resolved(tmp_path: Path) -> None: cfg = AlertConfig(transport="mailgun", smtp=None, mailgun=_mailgun_config()) with patch("door_sync.alert.httpx.post") as mock_post: - mock_post.return_value.raise_for_status = lambda: None alert.clear(path=path, alert_config=cfg) mock_post.assert_called_once() + mock_post.return_value.raise_for_status.assert_called_once() assert mock_post.call_args.kwargs["data"]["subject"] == "[door-sync] RESOLVED" assert not path.exists() @@ -183,9 +185,11 @@ def test_raise_smtp_sends_email(tmp_path: Path) -> None: cfg = AlertConfig(transport="smtp", smtp=_smtp_config(), mailgun=None) with patch("door_sync.alert.smtplib.SMTP") as mock_cls: + # mock_cls.return_value is a MagicMock, which already supports the + # context-manager protocol (and __exit__ returns False, so it won't + # swallow exceptions). _send_smtp uses `with server:` (no `as`), so no + # __enter__/__exit__ setup is needed. mock_server = mock_cls.return_value - mock_server.__enter__ = lambda s: s - mock_server.__exit__ = lambda s, *a: None alert.raise_("safety halt", path=path, alert_config=cfg) mock_server.starttls.assert_called_once() @@ -202,9 +206,11 @@ def test_clear_smtp_sends_resolved(tmp_path: Path) -> None: cfg = AlertConfig(transport="smtp", smtp=_smtp_config(), mailgun=None) with patch("door_sync.alert.smtplib.SMTP") as mock_cls: + # mock_cls.return_value is a MagicMock, which already supports the + # context-manager protocol (and __exit__ returns False, so it won't + # swallow exceptions). _send_smtp uses `with server:` (no `as`), so no + # __enter__/__exit__ setup is needed. mock_server = mock_cls.return_value - mock_server.__enter__ = lambda s: s - mock_server.__exit__ = lambda s, *a: None alert.clear(path=path, alert_config=cfg) msg = mock_server.send_message.call_args.args[0] @@ -247,9 +253,11 @@ def test_smtp_ssl_used_when_starttls_false(tmp_path: Path) -> None: cfg = AlertConfig(transport="smtp", smtp=smtp_cfg, mailgun=None) with patch("door_sync.alert.smtplib.SMTP_SSL") as mock_cls: + # mock_cls.return_value is a MagicMock, which already supports the + # context-manager protocol (and __exit__ returns False, so it won't + # swallow exceptions). _send_smtp uses `with server:` (no `as`), so no + # __enter__/__exit__ setup is needed. mock_server = mock_cls.return_value - mock_server.__enter__ = lambda s: s - mock_server.__exit__ = lambda s, *a: None alert.raise_("reason", path=path, alert_config=cfg) mock_cls.assert_called_once() diff --git a/tests/test_unifi_client.py b/tests/test_unifi_client.py index b8f3198..328774d 100644 --- a/tests/test_unifi_client.py +++ b/tests/test_unifi_client.py @@ -3,7 +3,9 @@ import hashlib import json as _json import logging +import ssl from collections.abc import Callable, Iterator +from dataclasses import replace from pathlib import Path from typing import Any from unittest.mock import MagicMock, patch @@ -12,7 +14,7 @@ import pytest from pytest_httpx import HTTPXMock -from door_sync.config import UnifiConfig +from door_sync.config import UnifiConfig, load from door_sync.models import Diff, ResolvedMember, UnifiUser from door_sync.unifi.client import ( UnifiClient, @@ -84,7 +86,10 @@ def test_parse_sync_alias_rejects_non_digit_suffix() -> None: assert _parse_sync_alias("sync-+5") is None assert _parse_sync_alias("sync- 12") is None # whitespace-padded assert _parse_sync_alias("sync-1_234") is None # underscore digit grouping - assert _parse_sync_alias("sync-²") is None # superscript two (unicode digit) + # Non-ASCII digits (str.isdigit() is True for these) must be rejected by the + # isascii() guard: a superscript and a regular non-ASCII decimal digit. + assert _parse_sync_alias("sync-²") is None # U+00B2 superscript two + assert _parse_sync_alias("sync-১") is None # U+09E7 Bengali digit one # --- Name splitting --- @@ -161,7 +166,12 @@ def _patched_tls(cert_der: bytes) -> Any: "door_sync.unifi.client", socket=MagicMock(create_connection=MagicMock(return_value=mock_sock)), ssl=MagicMock( - SSLContext=MagicMock(return_value=mock_ctx), CERT_NONE=0, PROTOCOL_TLS_CLIENT=0 + SSLContext=MagicMock(return_value=mock_ctx), + # Use the real ssl constants so the stub matches production values + # (e.g. PROTOCOL_TLS_CLIENT is 2, not 0) rather than magic numbers. + CERT_NONE=ssl.CERT_NONE, + PROTOCOL_TLS_CLIENT=ssl.PROTOCOL_TLS_CLIENT, + TLSVersion=ssl.TLSVersion, ), ) @@ -1006,8 +1016,9 @@ def test_import_cards_fc_mismatch_in_response_does_not_leak_card_number( client._import_cards([1234]) message = str(exc_info.value) # FC bytes are operational, not credential material — present. + # facility_code 42 (from the test config) must be named as the expected FC. assert "got FC 89" in message - assert f"expected {42}" in message + assert "expected 42" in message # The raw nfc_id and the card-number portion must NOT appear. assert "5904D2" not in message assert "1234" not in message @@ -1791,10 +1802,6 @@ def test_unifi_client_constructs_from_loaded_config( This catches the regression where config.host was validated as a full URL but the client was treating it as a bare hostname. """ - from dataclasses import replace - - from door_sync.config import load - repo_root = Path(__file__).parent.parent env_path = tmp_path / "env"