From f79c19dc56abf73288742e4632ec0cef8ea67cd9 Mon Sep 17 00:00:00 2001 From: QuantumFF Date: Sun, 7 Jun 2026 18:09:29 +0800 Subject: [PATCH] fix: detect skipped downloads despite streamrip's RichHandler line wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit streamrip logs the per-track skip line ("...Marked as downloaded in the database.") through rich's RichHandler, which wraps to the console width. When rip runs as a subprocess with a piped (non-tty) stdout that width defaults to 80 columns, so the phrase skip detection keys on wraps mid-line (with rich's right-aligned source location dropped into the gap). SKIP_LINE_RE then never matches and an already-downloaded album is misclassified as a fresh completion, losing its "already downloaded" state. Force a wide COLUMNS on the spawned rip via a new _rip_runner_env() helper (applied to both the download and search runners); the runner's per-line strip drops the resulting trailing padding. Add a regression test that drives the real runner through an actual RichHandler — the fake-runner tests feed clean single-line strings and so never exercised the wrap. Add rich to dev deps so that test runs in CI instead of skipping. Co-Authored-By: Claude Opus 4.8 (1M context) --- app.py | 16 ++++++++++++ requirements-dev.txt | 3 +++ tests/test_rip_runner_env.py | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 tests/test_rip_runner_env.py diff --git a/app.py b/app.py index 5b578a1..78c36a9 100644 --- a/app.py +++ b/app.py @@ -706,6 +706,20 @@ def parse_search_results(content, source, search_type): return ParsedSearch(results, None) +def _rip_runner_env(): + """Environment for a spawned `rip`, forcing a wide terminal width. + + streamrip logs through rich's RichHandler, which wraps to the console width; + when `rip` runs as a subprocess with a piped (non-tty) stdout that width + defaults to 80 columns. At 80 cols the single line skip detection keys on — + "...Marked as downloaded in the database." — wraps mid phrase, and rich drops + its right-aligned source location into the gap, so SKIP_LINE_RE never matches + and an already-downloaded album is misclassified as a fresh completion (its + "already downloaded" state is lost). A wide COLUMNS keeps each log line whole; + the per-line strip in the runner removes the resulting trailing padding.""" + return {**os.environ, "COLUMNS": "1000"} + + def _default_runner(cmd): """Run `rip` as a subprocess (ADR-0001), yielding stripped stdout lines and finally returning the exit code. This is the seam tests replace with a fake @@ -718,6 +732,7 @@ def _default_runner(cmd): encoding="utf-8", errors="replace", bufsize=1, + env=_rip_runner_env(), ) try: for line in process.stdout: @@ -749,6 +764,7 @@ def _default_search_runner(cmd): encoding="utf-8", errors="replace", timeout=30, + env=_rip_runner_env(), ) diff --git a/requirements-dev.txt b/requirements-dev.txt index b3b2b3c..b8e750b 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1 +1,4 @@ pytest==9.0.3 +# Used by tests/test_rip_runner_env.py to reproduce streamrip's RichHandler +# log wrapping; in production rich arrives transitively with streamrip. +rich diff --git a/tests/test_rip_runner_env.py b/tests/test_rip_runner_env.py new file mode 100644 index 0000000..64f8727 --- /dev/null +++ b/tests/test_rip_runner_env.py @@ -0,0 +1,50 @@ +"""The real `rip` runner forces a wide console so streamrip's RichHandler does +not wrap the skip log line. + +The fake-runner tests feed clean single-line strings, so they never exercise +the wrapping that broke skip detection in production: at the piped (non-tty) +default of 80 columns, "...Marked as downloaded in the database." wraps +mid-phrase (with rich's right-aligned source location dropped into the gap), +so SKIP_LINE_RE misses and an already-downloaded album is misclassified as a +fresh completion. These tests drive the actual subprocess runner through a +real RichHandler to lock that fix in. +""" +import sys +import textwrap + +import pytest + +import app as app_module +from app import classify_download + +# A child that logs exactly as streamrip does: basicConfig at INFO with a +# RichHandler, then the per-track "skip" line. RichHandler sizes itself from the +# COLUMNS the runner injects, so this wraps iff that width is narrow. +_RICH_SKIP_PROGRAM = textwrap.dedent( + """ + import logging + from rich.logging import RichHandler + logging.basicConfig(level="INFO", format="%(message)s", datefmt="[%X]", + handlers=[RichHandler()]) + log = logging.getLogger("streamrip") + log.setLevel(logging.INFO) + log.info("Skipping track 1234567890. Marked as downloaded in the database.") + """ +) + + +def test_runner_env_forces_wide_columns(): + env = app_module._rip_runner_env() + assert int(env["COLUMNS"]) >= 200 + # Base environment is preserved (PATH must survive so `rip` is found). + assert "PATH" in env + + +def test_skip_line_survives_rich_handler_unwrapped(): + pytest.importorskip("rich") + lines = list(app_module._default_runner([sys.executable, "-c", _RICH_SKIP_PROGRAM])) + output = "\n".join(lines) + # The skip phrase stays contiguous on one line, so detection classifies the + # run as skipped rather than completed. + assert app_module.SKIP_LINE_RE.search(output), output + assert classify_download(0, output) == "skipped"