Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -749,6 +764,7 @@ def _default_search_runner(cmd):
encoding="utf-8",
errors="replace",
timeout=30,
env=_rip_runner_env(),
)


Expand Down
3 changes: 3 additions & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -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
50 changes: 50 additions & 0 deletions tests/test_rip_runner_env.py
Original file line number Diff line number Diff line change
@@ -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"