diff --git a/.gitignore b/.gitignore index 725843f..77b66ea 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ __pycache__/ *.pyc +.pytest_cache/ *.db *.db-journal *.db-wal diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..dd8fa27 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,4 @@ +# Test-only dependencies. Not installed in the runtime image; used when running +# the test suite. See tests/README.md. +-r requirements.txt +pytest==8.4.2 diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..78246b9 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,44 @@ +# Tests + +Unit and safety tests for the control-plane app and the dedup file-deleter. +They run against throwaway temp dirs and a temp SQLite database (set up in +`conftest.py`); they never touch a real library, your `/config`, or the network. + +## Running them + +The runtime image doesn't ship `pytest`, so install it into a throwaway run of +the built image (which already has the app's own dependencies). Run as root so +pip can write into the image's venv, and keep pytest's cache off so it doesn't +create files in your working tree: + +```bash +docker run --rm --user root -v "$PWD":/src -w /src -e PYTHONPATH=/src \ + --entrypoint bash alembic:latest \ + -c "pip install -q pytest && pytest -q -p no:cacheprovider" +``` + +Tests set their own temp `ALEMBIC_CONFIG_DIR`, `MUSIC_DATA_DIR`, and encryption +key (see `conftest.py`), so nothing here touches a real library or config. + +Or, if you have a matching Python environment locally: + +```bash +pip install -r requirements-dev.txt +PYTHONPATH=. pytest -q +``` + +## What's covered + +- **Dedup safety** (`test_dedup_gating.py`): `dedup-library.sh` deletes only the + paths confirmed via `--only-paths`, and keeps the higher-ranked (FLAC) copy. + This is the most important test: the script deletes files. +- **Runner** (`test_pipeline_runner.py`): lock skip, `use_lock=False` bypass, + success/failure/timeout recording, lock release. +- **Migrations** (`test_db.py`): schema version, idempotent re-run, FK-safe + job-run pruning, stale-run reconciliation. +- **Credential rendering** (`test_credential_service.py`): config field + substitution (including newline-injection safety), secret vs non-secret + classification, cookie-expiry parsing. +- **Playlists** (`test_playlist_service.py`): cron round-trips, name validation, + config render/delete. +- **Diagnostics** (`test_diagnostics.py`): the `/health` and `/setup` checks. diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..7529c14 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,67 @@ +"""Test setup. + +Every path-derived setting must point at throwaway dirs BEFORE any app module +imports, because app.db builds its SQLAlchemy engine from settings at import +time. So this runs at module load (conftest is imported before test modules). +""" +import os +import tempfile +from pathlib import Path + +from cryptography.fernet import Fernet + +_REPO = Path(__file__).resolve().parent.parent +_TMP = Path(tempfile.mkdtemp(prefix="alembic-tests-")) +(_TMP / "config" / "pipeline").mkdir(parents=True, exist_ok=True) +(_TMP / "music").mkdir(parents=True, exist_ok=True) + +_KEY = _TMP / "master.key" +_KEY.write_bytes(Fernet.generate_key()) + +os.environ["ALEMBIC_CONFIG_DIR"] = str(_TMP / "config") +os.environ["MUSIC_DATA_DIR"] = str(_TMP / "music") +os.environ["PIPELINE_DIR"] = str(_REPO / "pipeline") +os.environ["ENCRYPTION_KEY_FILE"] = str(_KEY) +os.environ["SESSION_SECRET"] = "test-only-secret" +# Make sure no stray OIDC env leaks in from the host shell. +for _v in ("POCKETID_ISSUER", "POCKETID_CLIENT_ID", "POCKETID_CLIENT_SECRET", "ALLOWED_EMAIL"): + os.environ.pop(_v, None) + +import pytest # noqa: E402 + +_TABLES = [ + "job_runs", "dedup_candidates", "dedup_runs", "genre_candidates", "genre_runs", + "manual_imports", "manual_fix_audit", "playlists", "secrets", "scheduled_jobs", + "users", "app_settings", +] + + +@pytest.fixture(autouse=True) +def fresh_db(): + """Each test gets an initialized, empty database (the engine is bound to the + one temp DB path, so clear rows between tests rather than rebinding).""" + from sqlalchemy import text + + from app.db import engine, init_db + + init_db() + with engine.begin() as conn: + for table in _TABLES: + conn.execute(text(f"DELETE FROM {table}")) + yield + + +@pytest.fixture() +def db(): + from app.db import SessionLocal + + session = SessionLocal() + try: + yield session + finally: + session.close() + + +@pytest.fixture() +def config_dir(): + return _TMP / "config" diff --git a/tests/test_credential_service.py b/tests/test_credential_service.py new file mode 100644 index 0000000..563a6d2 --- /dev/null +++ b/tests/test_credential_service.py @@ -0,0 +1,59 @@ +import os +import stat + +from app.services import credential_service as cs +from app.services import playlist_service as pl + + +def test_set_conf_field_replaces_existing(): + text = "user = OLD\npass = OLD\n" + out = cs._set_conf_field(text, "user", "newuser") + assert "user = newuser" in out + assert "pass = OLD" in out # other lines untouched + + +def test_set_conf_field_appends_when_absent(): + out = cs._set_conf_field("input = x\n", "user", "bob") + assert out.rstrip().endswith("user = bob") + + +def test_set_conf_field_strips_newlines(): + # a newline in a value must not inject a second config line + out = cs._set_conf_field("user = x\n", "user", "bad\nname = injected") + assert "\nname = injected" not in out + assert "user = badname = injected" in out + + +def test_secret_keys_classification(): + assert "password" in cs.SECRET_KEYS + assert "client_secret" in cs.SECRET_KEYS + # identifiers/urls/prefs are NOT secret (shown as text in the UI) + assert "username" not in cs.SECRET_KEYS + assert "base_url" not in cs.SECRET_KEYS + assert "region" not in cs.SECRET_KEYS + + +def test_render_playlist_confs_injects_creds_safely(db, config_dir): + cs.set_credentials(db, "spotify", {"client_id": "CID", "client_secret": "CSECRET"}) + # a password containing shell metacharacters must land literally + cs.set_credentials(db, "soulseek", {"username": "seek", "password": "p'a$(id)ss"}) + pl.create(db, name="rtest", spotify_url="https://open.spotify.com/playlist/Z") + + conf = config_dir / "pipeline" / "rtest.conf" + text = conf.read_text() + assert "user = seek" in text + assert "pass = p'a$(id)ss" in text + assert "spotify-id = CID" in text + assert "spotify-secret = CSECRET" in text + # rendered config must be owner-only (holds the Soulseek password) + assert stat.S_IMODE(os.stat(conf).st_mode) == 0o600 + + +def test_bandcamp_cookie_expiry_parsing(): + jar = "\n".join([ + "# Netscape HTTP Cookie File", + "\t".join([".bandcamp.com", "TRUE", "/", "TRUE", "1893456000", "identity", "abc"]), + ]) + exp = cs._bandcamp_cookie_expiry(jar) + assert exp == 1893456000.0 + assert cs._bandcamp_cookie_expiry("no identity cookie here") is None diff --git a/tests/test_db.py b/tests/test_db.py new file mode 100644 index 0000000..99135ce --- /dev/null +++ b/tests/test_db.py @@ -0,0 +1,61 @@ +import time + +from sqlalchemy import text + +from app import db as appdb + + +def test_init_db_sets_version_and_is_idempotent(): + with appdb.engine.begin() as conn: + version = conn.execute(text("SELECT version FROM schema_version")).fetchone()[0] + assert version >= 3 # all shipped migrations applied + # Re-running must not raise (e.g. duplicate-column on a re-applied ALTER). + appdb.init_db() + appdb.init_db() + + +def test_prune_old_job_runs_fk_safe(): + now = time.time() + old = now - 100 * 86400 + with appdb.engine.begin() as conn: + conn.execute( + text( + "INSERT INTO job_runs (id, job_key, started_at, status, triggered_by) " + "VALUES (1,'old',:o,'success','schedule'),(2,'new',:n,'success','schedule')" + ), + {"o": old, "n": now}, + ) + conn.execute( + text( + "INSERT INTO manual_imports (id, source_path, imported_by, imported_at, status, job_run_id) " + "VALUES (1,'x','me',:o,'done',1)" + ), + {"o": old}, + ) + + deleted = appdb.prune_old_job_runs(max_age_days=90) + assert deleted == 1 + with appdb.engine.begin() as conn: + remaining = [r[0] for r in conn.execute(text("SELECT job_key FROM job_runs"))] + mi_ref = conn.execute(text("SELECT job_run_id FROM manual_imports WHERE id=1")).fetchone()[0] + mi_count = conn.execute(text("SELECT COUNT(*) FROM manual_imports")).fetchone()[0] + assert remaining == ["new"] + assert mi_ref is None # reference nulled, not cascade-deleted + assert mi_count == 1 # the import record itself is kept + + +def test_mark_interrupted_runs(): + now = time.time() + with appdb.engine.begin() as conn: + conn.execute( + text( + "INSERT INTO job_runs (id, job_key, started_at, status, triggered_by) " + "VALUES (1,'stuck',:n,'running','schedule')" + ), + {"n": now}, + ) + appdb.mark_interrupted_runs() + with appdb.engine.begin() as conn: + row = conn.execute(text("SELECT status, finished_at FROM job_runs WHERE id=1")).fetchone() + assert row[0] == "interrupted" + assert row[1] is not None diff --git a/tests/test_dedup_gating.py b/tests/test_dedup_gating.py new file mode 100644 index 0000000..da4407b --- /dev/null +++ b/tests/test_dedup_gating.py @@ -0,0 +1,53 @@ +"""The safety-critical test: dedup-library.sh must delete ONLY the files a +human confirmed via --only-paths, and must keep the higher-ranked copy. + +Rather than stand up a full beets fixture, this sources the script's setup + +function definitions (everything before the Pass 1 top-level code) and drives +process_group directly with "ghost" (bare-path) entries, which skip the beets +path translation and are deleted with plain rm -- exactly the code path that +enforces --only-paths gating and FLAC-over-MP3 ranking.""" +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +SCRIPT = REPO / "pipeline" / "lib" / "dedup-library.sh" + + +def _drive_process_group(work: Path) -> subprocess.CompletedProcess: + keep = work / "keep.flac" + dupe = work / "dupe.mp3" + other = work / "other.mp3" + for f, size in ((keep, 5000), (dupe, 1000), (other, 1000)): + f.write_bytes(b"\0" * size) + confirm = work / "confirm.txt" + confirm.write_text(str(dupe) + "\n") # only dupe.mp3 is confirmed for deletion + + script = r""" +set -euo pipefail +export ALEMBIC_CONFIG_DIR="{work}" MUSIC_DATA_DIR="{work}" +mkdir -p "{work}/logs" +# Source only setup + functions (stop before the Pass 1 top-level code). +end=$(grep -n '^# Pass 1: numbered siblings' "{script}" | head -1 | cut -d: -f1) +source <(head -n $((end - 2)) "{script}") +APPLY=1 +ONLY_PATHS_FILE="{confirm}" +declare -A ONLY_PATHS=(["{dupe}"]=1) +# Group 1: keep.flac vs dupe.mp3 (dupe confirmed -> deleted) +process_group "P2 case-insensitive: g1" "{keep}" "{dupe}" +# Group 2: keep.flac vs other.mp3 (other NOT confirmed -> must survive) +process_group "P2 case-insensitive: g2" "{keep}" "{other}" +""".format(work=work, script=SCRIPT, confirm=confirm, dupe=dupe, keep=keep, other=other) + return subprocess.run(["bash", "-c", script], capture_output=True, text=True) + + +def test_only_confirmed_paths_are_deleted(tmp_path): + proc = _drive_process_group(tmp_path) + assert proc.returncode == 0, proc.stderr + + keep = tmp_path / "keep.flac" + dupe = tmp_path / "dupe.mp3" + other = tmp_path / "other.mp3" + + assert keep.exists(), "FLAC (higher rank) must be kept" + assert not dupe.exists(), "confirmed duplicate must be deleted" + assert other.exists(), "an unconfirmed delete-candidate must NOT be deleted" diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py new file mode 100644 index 0000000..f94e8aa --- /dev/null +++ b/tests/test_diagnostics.py @@ -0,0 +1,25 @@ +from app.services import diagnostics + + +def test_master_key_present(): + # conftest wrote a real key at ENCRYPTION_KEY_FILE + assert diagnostics.master_key_present() is True + + +def test_oidc_not_configured_by_default(): + assert diagnostics.oidc_configured() is False + + +def test_summary_degraded_without_oidc(): + result = diagnostics.summary() + assert result["status"] == "degraded" # OIDC missing is critical + keys = {c["key"] for c in result["checks"]} + assert {"oidc", "master_key", "config_writable"} <= keys + + +def test_config_writable_true(): + checks = {c["key"]: c for c in diagnostics.checks()} + assert checks["config_writable"]["ok"] is True + # critical checks are flagged as such + assert checks["oidc"]["critical"] is True + assert checks["beets_library"]["critical"] is False diff --git a/tests/test_pipeline_runner.py b/tests/test_pipeline_runner.py new file mode 100644 index 0000000..3415d70 --- /dev/null +++ b/tests/test_pipeline_runner.py @@ -0,0 +1,60 @@ +import asyncio + +from app.services import pipeline_runner as pr + +# One loop for the whole module: pipeline_runner._lock is a module-level +# asyncio.Lock, and reusing it across different loops (as asyncio.run would +# create) raises "bound to a different event loop". +_loop = asyncio.new_event_loop() + + +def _run(coro): + return _loop.run_until_complete(coro) + + +def test_run_job_success_records_row_and_log(): + run = _run(pr.run_job("test:ok", ["true"])) + assert run.status == "success" + assert run.exit_code == 0 + assert run.log_path # a log file path was recorded + + +def test_run_job_failure(): + run = _run(pr.run_job("test:fail", ["false"])) + assert run.status == "failed" + assert run.exit_code == 1 + + +def test_lock_skip_when_held(): + async def scenario(): + await pr._lock.acquire() + try: + return await pr.run_job("test:locked", ["true"], use_lock=True) + finally: + pr._lock.release() + + run = _run(scenario()) + assert run.status == "skipped_lock" + + +def test_use_lock_false_bypasses_held_lock(): + async def scenario(): + await pr._lock.acquire() + try: + return await pr.run_job("test:bypass", ["true"], use_lock=False) + finally: + pr._lock.release() + + run = _run(scenario()) + assert run.status == "success" + + +def test_lock_released_after_run(): + _run(pr.run_job("test:release", ["true"])) + assert not pr._lock.locked() + + +def test_timeout_kills_and_marks_failed(): + run = _run(pr.run_job("test:timeout", ["sleep", "5"], timeout=0.3)) + assert run.status == "failed" + assert run.exit_code == -1 diff --git a/tests/test_playlist_service.py b/tests/test_playlist_service.py new file mode 100644 index 0000000..08b9edb --- /dev/null +++ b/tests/test_playlist_service.py @@ -0,0 +1,51 @@ +import pytest + +from app.services import playlist_service as pl + + +@pytest.mark.parametrize( + "time_str,cron", + [("01:30", "30 1 * * *"), ("00:00", "0 0 * * *"), ("23:59", "59 23 * * *")], +) +def test_cron_roundtrip(time_str, cron): + assert pl.time_to_cron(time_str) == cron + assert pl.cron_to_time(cron) == time_str + + +def test_time_to_cron_invalid(): + assert pl.time_to_cron("") is None + assert pl.time_to_cron("25:00") is None + assert pl.time_to_cron("nonsense") is None + + +def test_cron_to_time_non_daily_is_blank(): + # weekly/day-of-month crons aren't representable as a plain HH:MM + assert pl.cron_to_time("30 1 * * sun") == "" + assert pl.cron_to_time(None) == "" + + +@pytest.mark.parametrize("name", ["techno", "my chill mix", "goldenera", "a-b_c 1"]) +def test_validate_name_accepts(name): + assert pl.validate_name(name) == name.strip() + + +@pytest.mark.parametrize("name", ["", "../escape", "a/b", "x;rm -rf", "x|e", "ΓΌ", "x" * 65]) +def test_validate_name_rejects(name): + with pytest.raises(ValueError): + pl.validate_name(name) + + +def test_create_renders_conf_and_delete_removes_it(db, config_dir): + p = pl.create(db, name="unittest", spotify_url="https://open.spotify.com/playlist/ABC") + conf = config_dir / "pipeline" / "unittest.conf" + assert conf.exists() + text = conf.read_text() + assert "input = https://open.spotify.com/playlist/ABC" in text + assert "path = " in text and "/unittest" in text + pl.delete(db, p.id) + assert not conf.exists() + + +def test_create_rejects_bad_name(db): + with pytest.raises(ValueError): + pl.create(db, name="../evil", spotify_url="https://open.spotify.com/playlist/X")