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"