Hardening pass: scheduler backoff, FK cascade, a11y, test safety net
Pre-traffic cleanup from an audit: * Scheduler: poll_due_sources now keys on the last *attempt* (success or failure), not the last success, and scales the wait by the consecutive- failure streak (capped at a day). A failing feed (e.g. Phys.org's HTTP 429s) used to be retried every cycle because it had no successful run; it now backs off and recovers on its own. Extracted due_source_rows() + tests. * FK hygiene: deleting a daily_brief is supposed to cascade to its items, but SQLite enforces foreign keys per-connection — connect() already sets the pragma, so the cascade is correct going forward; added a regression test. (Orphaned items + Phys.org settings were cleaned directly on the live DB.) * a11y: modal/drawer dialogs are now focusable (tabindex), close on Escape (window) and on backdrop click via a target check (dropping the inner stopPropagation handlers). Build is warning-free. * tests: conftest points any un-mocked LLM client at a closed port with a 1s timeout, so an accidental real call fails fast instead of hanging the suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,16 @@
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_real_llm(monkeypatch):
|
||||
"""Safety net: no test should reach the real arbiter.
|
||||
|
||||
If a test constructs an LLM client from env without mocking, point it at a
|
||||
closed local port with a 1s timeout so the call fails fast instead of
|
||||
hanging on the production 300s timeout (which looks like a stalled suite).
|
||||
Tests that exercise the LLM patch it at the function level, which overrides
|
||||
these env defaults.
|
||||
"""
|
||||
monkeypatch.setenv("GOODNEWS_LLM_BASE_URL", "http://127.0.0.1:1/v1")
|
||||
monkeypatch.setenv("GOODNEWS_LLM_TIMEOUT", "1")
|
||||
monkeypatch.delenv("GOODNEWS_LLM_API_KEY", raising=False)
|
||||
@@ -0,0 +1,25 @@
|
||||
from goodnews.db import connect, init_db
|
||||
|
||||
|
||||
def test_deleting_a_brief_cascades_to_items(tmp_path):
|
||||
"""The orphaned-items bug: a brief was deleted while its items survived,
|
||||
because FK enforcement is per-connection in SQLite. connect() turns it on,
|
||||
so ON DELETE CASCADE must now clear the items and leave no FK violations.
|
||||
"""
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
assert c.execute("PRAGMA foreign_keys").fetchone()[0] == 1 # enforced
|
||||
|
||||
c.execute("INSERT INTO sources (id, name, feed_url) VALUES (1, 'S', 'http://s/f')")
|
||||
c.execute(
|
||||
"INSERT INTO articles (id, source_id, canonical_url, title, url_hash, published_at) "
|
||||
"VALUES (1, 1, 'http://s/a', 'A', 'h1', '2026-01-01T00:00:00')"
|
||||
)
|
||||
c.execute("INSERT INTO daily_briefs (id, brief_date, title) VALUES (9, '2026-01-01', 'B')")
|
||||
c.execute("INSERT INTO daily_brief_items (brief_id, article_id, rank) VALUES (9, 1, 1)")
|
||||
c.commit()
|
||||
|
||||
c.execute("DELETE FROM daily_briefs WHERE id = 9")
|
||||
c.commit()
|
||||
|
||||
assert c.execute("SELECT COUNT(*) FROM daily_brief_items").fetchone()[0] == 0
|
||||
assert c.execute("PRAGMA foreign_key_check").fetchall() == []
|
||||
@@ -0,0 +1,62 @@
|
||||
from goodnews.db import connect, init_db
|
||||
from goodnews.feeds import due_source_rows
|
||||
|
||||
|
||||
def _src(c, sid, *, interval=120, failures=0, active=1):
|
||||
c.execute(
|
||||
"INSERT INTO sources (id, name, feed_url, poll_interval_minutes, consecutive_failures, active) "
|
||||
"VALUES (?, ?, ?, ?, ?, ?)",
|
||||
(sid, f"S{sid}", f"http://s/{sid}", interval, failures, active),
|
||||
)
|
||||
|
||||
|
||||
def _attempt(c, sid, *, minutes_ago, status="ok"):
|
||||
c.execute(
|
||||
"INSERT INTO ingest_runs (source_id, finished_at, status) "
|
||||
"VALUES (?, datetime('now', ?), ?)",
|
||||
(sid, f"-{minutes_ago} minutes", status),
|
||||
)
|
||||
|
||||
|
||||
def _due_ids(c):
|
||||
return {r["id"] for r in due_source_rows(c)}
|
||||
|
||||
|
||||
def test_never_attempted_is_due(tmp_path):
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
_src(c, 1)
|
||||
assert _due_ids(c) == {1}
|
||||
|
||||
|
||||
def test_recent_attempt_not_due_old_attempt_due(tmp_path):
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
_src(c, 1, interval=120); _attempt(c, 1, minutes_ago=5) # polled recently
|
||||
_src(c, 2, interval=120); _attempt(c, 2, minutes_ago=200) # overdue
|
||||
assert _due_ids(c) == {2}
|
||||
|
||||
|
||||
def test_failing_source_backs_off(tmp_path):
|
||||
# The regression: a perpetually-FAILING source (no 'ok' run) used to be due
|
||||
# every cycle. With backoff it rests — effective wait = 60*(1+10)=660 min.
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
_src(c, 1, interval=60, failures=10)
|
||||
_attempt(c, 1, minutes_ago=200, status="failed") # 200 < 660 → not due yet
|
||||
assert _due_ids(c) == set()
|
||||
# ...but once past the backed-off window it becomes due again.
|
||||
c.execute("DELETE FROM ingest_runs WHERE source_id = 1")
|
||||
_attempt(c, 1, minutes_ago=700, status="failed") # 700 > 660 → due
|
||||
assert _due_ids(c) == {1}
|
||||
|
||||
|
||||
def test_backoff_capped_at_a_day(tmp_path):
|
||||
# A long failure streak can't push the wait past MAX_BACKOFF_MINUTES (1440).
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
_src(c, 1, interval=120, failures=999) # 120*1000 would be huge
|
||||
_attempt(c, 1, minutes_ago=1500, status="failed") # >1440 → due despite streak
|
||||
assert _due_ids(c) == {1}
|
||||
|
||||
|
||||
def test_inactive_never_due(tmp_path):
|
||||
c = connect(str(tmp_path / "t.db")); init_db(c)
|
||||
_src(c, 1, active=0)
|
||||
assert _due_ids(c) == set()
|
||||
Reference in New Issue
Block a user