fix: handle ValueError from parse_occurrence and add edge-case occurrence tests
- Wrap parse_occurrence() call in try/except ValueError; return skipped with reason instead of crashing
- Remove redundant `or {}` guard on sub_row (already defaulted to {} via next(..., {}))
- Strengthen test_occurrence_passes assertion to check status == "success"
- Add 3 edge-case tests: occurrence="every", missing occurrence key, invalid occurrence string
This commit is contained in:
parent
08aa019439
commit
a3932aef1e
2 changed files with 81 additions and 3 deletions
|
|
@ -39,8 +39,11 @@ def _run_post(db_path: str, campaign_id: int, target: str,
|
||||||
sub_row = next((s for s in all_subs if s["sub"] == target), {})
|
sub_row = next((s for s in all_subs if s["sub"] == target), {})
|
||||||
|
|
||||||
# Occurrence check — skip if not the right week of the month
|
# Occurrence check — skip if not the right week of the month
|
||||||
occurrence_str = (sub_row or {}).get("occurrence")
|
occurrence_str = sub_row.get("occurrence")
|
||||||
|
try:
|
||||||
parsed = parse_occurrence(occurrence_str)
|
parsed = parse_occurrence(occurrence_str)
|
||||||
|
except ValueError as exc:
|
||||||
|
return {"skipped": True, "reason": f"invalid occurrence {occurrence_str!r}: {exc}"}
|
||||||
if parsed is not None:
|
if parsed is not None:
|
||||||
weekday, n = parsed
|
weekday, n = parsed
|
||||||
if not is_nth_weekday(date.today(), weekday, n):
|
if not is_nth_weekday(date.today(), weekday, n):
|
||||||
|
|
|
||||||
|
|
@ -160,5 +160,80 @@ def test_occurrence_passes(tmp_path):
|
||||||
mock_date.today.return_value = real_date(2026, 5, 3)
|
mock_date.today.return_value = real_date(2026, 5, 3)
|
||||||
result = _run_post(db, campaign_id=1, target="selfhosted", triggered_by="scheduler")
|
result = _run_post(db, campaign_id=1, target="selfhosted", triggered_by="scheduler")
|
||||||
|
|
||||||
assert result.get("skipped") is not True
|
assert result.get("status") == "success"
|
||||||
mock_strategy.execute.assert_called_once()
|
mock_strategy.execute.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_occurrence_every_passes_through(tmp_path):
|
||||||
|
"""occurrence='every' means post every time — no filtering, execute is called."""
|
||||||
|
db = str(tmp_path / "test.db")
|
||||||
|
# parse_occurrence("every") returns None → no filtering → execute runs
|
||||||
|
mock_store = _make_store(
|
||||||
|
campaign_type="reddit_comment",
|
||||||
|
subs=[{"sub": "selfhosted", "active": 1, "occurrence": "every"}],
|
||||||
|
)
|
||||||
|
mock_result = MagicMock()
|
||||||
|
mock_result.url = "https://reddit.com/r/selfhosted/comments/abc/"
|
||||||
|
|
||||||
|
mock_strategy = MagicMock()
|
||||||
|
mock_strategy.supports_dupe_guard.return_value = False
|
||||||
|
mock_strategy.execute.return_value = mock_result
|
||||||
|
|
||||||
|
with patch("app.services.poster.Store", return_value=mock_store):
|
||||||
|
with patch("app.services.poster.get_client", return_value=mock_strategy):
|
||||||
|
with patch("app.services.poster.date") as mock_date:
|
||||||
|
from datetime import date as real_date
|
||||||
|
mock_date.today.return_value = real_date(2026, 5, 3)
|
||||||
|
result = _run_post(db, campaign_id=1, target="selfhosted", triggered_by="scheduler")
|
||||||
|
|
||||||
|
assert result.get("status") == "success"
|
||||||
|
mock_strategy.execute.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_occurrence_none_sub_row_key_passes_through(tmp_path):
|
||||||
|
"""Sub row exists but has no occurrence key — should post normally."""
|
||||||
|
db = str(tmp_path / "test.db")
|
||||||
|
# sub_row has no "occurrence" key → .get() returns None → parse_occurrence(None) returns None
|
||||||
|
mock_store = _make_store(
|
||||||
|
campaign_type="reddit_comment",
|
||||||
|
subs=[{"sub": "selfhosted", "active": 1}],
|
||||||
|
)
|
||||||
|
mock_result = MagicMock()
|
||||||
|
mock_result.url = "https://reddit.com/r/selfhosted/comments/abc/"
|
||||||
|
|
||||||
|
mock_strategy = MagicMock()
|
||||||
|
mock_strategy.supports_dupe_guard.return_value = False
|
||||||
|
mock_strategy.execute.return_value = mock_result
|
||||||
|
|
||||||
|
with patch("app.services.poster.Store", return_value=mock_store):
|
||||||
|
with patch("app.services.poster.get_client", return_value=mock_strategy):
|
||||||
|
with patch("app.services.poster.date") as mock_date:
|
||||||
|
from datetime import date as real_date
|
||||||
|
mock_date.today.return_value = real_date(2026, 5, 3)
|
||||||
|
result = _run_post(db, campaign_id=1, target="selfhosted", triggered_by="scheduler")
|
||||||
|
|
||||||
|
assert result.get("status") == "success"
|
||||||
|
mock_strategy.execute.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_occurrence_invalid_string_skips(tmp_path):
|
||||||
|
"""Malformed occurrence string results in skipped=True, not a crash."""
|
||||||
|
db = str(tmp_path / "test.db")
|
||||||
|
# "weekly" is not a valid occurrence string — parse_occurrence raises ValueError
|
||||||
|
mock_store = _make_store(
|
||||||
|
campaign_type="reddit_comment",
|
||||||
|
subs=[{"sub": "selfhosted", "active": 1, "occurrence": "weekly"}],
|
||||||
|
)
|
||||||
|
mock_strategy = MagicMock()
|
||||||
|
mock_strategy.supports_dupe_guard.return_value = False
|
||||||
|
|
||||||
|
with patch("app.services.poster.Store", return_value=mock_store):
|
||||||
|
with patch("app.services.poster.get_client", return_value=mock_strategy):
|
||||||
|
with patch("app.services.poster.date") as mock_date:
|
||||||
|
from datetime import date as real_date
|
||||||
|
mock_date.today.return_value = real_date(2026, 5, 3)
|
||||||
|
result = _run_post(db, campaign_id=1, target="selfhosted", triggered_by="scheduler")
|
||||||
|
|
||||||
|
assert result.get("skipped") is True
|
||||||
|
assert "invalid occurrence" in result.get("reason", "")
|
||||||
|
mock_strategy.execute.assert_not_called()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue