v0.20.4: the song that wasn't there

trav couldn't play the first track of his playlist. Shuffle off gave him song two; shuffle on gave him a random one. The player was right both times: library.json named a song whose file was not on the device, and skipping an unplayable file advances one slot. Tapping any song that *is* there plays exactly that song, shuffle or not. The file was missing because a gvfs-MTP mount can hold a phantom directory -- one it lists happily while the device has no such folder. Every write into it fails EIO, and mkdir(exist_ok=True) sees the phantom and does nothing, so it never heals; only remounting clears it. That folder was new because Round 52's retag moved the file under a new artist. So a copy that raises OSError now costs that song, not the sync, and the song is left out of the m3u and library.json. Aborting cost 2,400 songs for one folder; counting it present anyway put a song in the manifest that isn't on the device, which is the bug you could hear. The names come back in the summary and a dialog rather than vanishing. Planning is a worker now. plan_andtunes_sync is pure and writes nothing, but it walks every file on the device, and over MTP that is thousands of round trips -- on the GUI thread it froze the window and GNOME offered to kill LinTunes, which is how a sync got force-quit halfway through. And the sync stopped going quiet at the end. "Album art 501/501" is emitted before the last album, and then _write_index, _ship_buttons and _prune_empty_dirs ran silently: 501 exists() stats over MTP, ~840 KB of writes and a full tree walk, with the progress line frozen. They report now, and _write_index reads Art/ in one listing instead of a stat per album. andTunes 0.2.2: the wheel scrolls the other way in lists, and smoothly -- a detent adds to a pixel debt that a Choreographer callback pays off a fraction per frame, so one detent eases to a stop and a fast spin blends into one movement instead of teleporting a row at a time. Volume is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SKXUgsBBwe3qaHEjeV8ubP
2026-09-14 13:25:15 -07:00
parent def7635a87
commit efa07b05c6
12 changed files with 457 additions and 36 deletions
@@ -0,0 +1,186 @@
+"""Round 55: a song the device wouldn't take, and a window that stayed alive.
+
+trav's Rabbit played the wrong song whenever he tapped the first track of his
+playlist — shuffle off gave him song 2, shuffle on gave him a random one. The
+player was right both times: `library.json` listed a song whose file was not
+on the device, and skipping an unplayable file advances one slot.
+
+The file was missing because a gvfs-MTP mount can hold a **phantom directory**
+— one it lists but the device doesn't have. Every write into it fails EIO, and
+`mkdir(exist_ok=True)` sees the phantom and does nothing, so it never heals.
+Two things were wrong with how sync met that: it aborted the whole run, and
+(worse, when it didn't) the track still went into the manifest.
+
+What's pinned:
+
+* a copy that raises OSError costs **that song**, not the sync, and the song
+  is left out of the m3u and `library.json` — the app can only list what it
+  can play — and comes back in the summary so it isn't silent;
+* planning is a **worker**, because it walks every file on the device and over
+  MTP that froze the window long enough for GNOME to offer to kill LinTunes;
+* the phases after the last album (`_write_index`, buttons, pruning) **say so**
+  — they were minutes of silence with the progress line stuck on the last
+  album, which reads exactly like a hang.
+"""
+
+import json
+
+import pytest
+
+from lintunes.andtunes.sync import (
+    AndTunesPlanWorker, AndTunesSyncWorker, plan_andtunes_sync,
+)
+from lintunes.models import Track
+from lintunes.models.playlist import Playlist, PlaylistType
+
+
+@pytest.fixture(autouse=True)
+def _private_cache(tmp_path, monkeypatch):
+    monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path / "cache"))
+
+
+DATA = b"x" * 64
+
+
+def _track(tid, name, artist="Band", album="Record", *, tmp_path):
+    path = tmp_path / "local" / f"{tid}.mp3"
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_bytes(DATA)
+    return Track(track_id=tid, name=name, artist=artist, album=album,
+                 location=str(path), total_time=60_000, kind="MPEG audio file")
+
+
+def _playlist(*tids):
+    return Playlist(name="Mix", persistent_id="AAAA0001",
+                    playlist_type=PlaylistType.REGULAR, track_ids=list(tids))
+
+
+def _root(tmp_path):
+    return tmp_path / "device" / "Music" / "andTunes"
+
+
+def _run(plan):
+    worker = AndTunesSyncWorker(plan)
+    out = {"labels": []}
+    worker.finished.connect(lambda s: out.setdefault("finished", s))
+    worker.failed.connect(lambda m: out.setdefault("failed", m))
+    worker.progress.connect(lambda done, total, label: out["labels"].append(label))
+    worker._run()
+    return out
+
+
+def _manifest(root):
+    return json.loads((root / "library.json").read_text(encoding="utf-8"))
+
+
+def _wedge(monkeypatch, bad_name: str):
+    """Make open(..., 'wb') raise EIO for one destination, like a phantom dir."""
+    real = open
+
+    def fake(file, mode="r", *a, **kw):
+        if "w" in str(mode) and str(file).endswith(bad_name):
+            raise OSError(5, "Input/output error", str(file))
+        return real(file, mode, *a, **kw)
+
+    monkeypatch.setattr("builtins.open", fake)
+
+
+# ---- one song the device refuses ----
+
+def test_a_refused_copy_costs_that_song_not_the_sync(tmp_path, monkeypatch):
+    tracks = {
+        1: _track(1, "Wanted", tmp_path=tmp_path),
+        2: _track(2, "Refused", tmp_path=tmp_path),
+        3: _track(3, "Also Wanted", tmp_path=tmp_path),
+    }
+    plan = plan_andtunes_sync([_playlist(1, 2, 3)], tracks, _root(tmp_path))
+    _wedge(monkeypatch, "Refused.mp3")
+
+    out = _run(plan)
+
+    assert "failed" not in out, out.get("failed")
+    summary = out["finished"]
+    assert summary["tracks"] == 2
+    assert [name for name, _why in summary["unwritable"]] == ["Band - Refused"]
+    assert "Input/output error" in summary["unwritable"][0][1]
+
+
+def test_a_refused_song_is_left_out_of_the_library_the_app_reads(
+        tmp_path, monkeypatch):
+    """The whole point: the app must never list a song it can't play."""
+    tracks = {
+        1: _track(1, "Wanted", tmp_path=tmp_path),
+        2: _track(2, "Refused", tmp_path=tmp_path),
+    }
+    root = _root(tmp_path)
+    plan = plan_andtunes_sync([_playlist(1, 2)], tracks, root)
+    _wedge(monkeypatch, "Refused.mp3")
+
+    _run(plan)
+
+    manifest = _manifest(root)
+    assert [t["id"] for t in manifest["tracks"]] == [1]
+    assert manifest["playlists"][0]["tracks"] == [1]
+    m3u = (root / "Playlists" / "Mix.m3u").read_text(encoding="utf-8")
+    assert "Refused" not in m3u
+    assert not (root / "Media/Band/Record/Refused.mp3").exists()
+
+
+def test_no_refusals_means_no_noise_in_the_summary(tmp_path):
+    tracks = {1: _track(1, "Fine", tmp_path=tmp_path)}
+    plan = plan_andtunes_sync([_playlist(1)], tracks, _root(tmp_path))
+
+    summary = _run(plan)["finished"]
+
+    assert summary["unwritable"] == []
+    assert summary["tracks"] == 1
+
+
+# ---- the tail of the sync says what it's doing ----
+
+def test_the_phases_after_the_art_report_progress(tmp_path):
+    tracks = {1: _track(1, "Fine", tmp_path=tmp_path)}
+    plan = plan_andtunes_sync([_playlist(1)], tracks, _root(tmp_path))
+
+    labels = _run(plan)["labels"]
+
+    assert any(l.startswith("Writing playlist") for l in labels), labels
+    assert "Writing the library" in labels, labels
+    assert "Tidying up" in labels, labels
+    # And the index phase is the *last* thing said, not a stale album count.
+    assert labels[-1] == "Tidying up"
+
+
+# ---- planning is a worker ----
+
+def test_planning_runs_off_the_gui_thread_and_reports_a_plan(tmp_path):
+    tracks = {1: _track(1, "Fine", tmp_path=tmp_path)}
+    worker = AndTunesPlanWorker(
+        [_playlist(1)], tracks, _root(tmp_path), None, copy_art=False)
+    got = {}
+    worker.planned.connect(lambda plan, free: got.update(plan=plan, free=free))
+    worker.failed.connect(lambda m: got.update(failed=m))
+
+    worker._run()
+
+    assert "failed" not in got, got.get("failed")
+    assert [i.track_id for i in got["plan"].items] == [1]
+    assert got["free"] == 0          # no device, so nothing to measure
+
+
+def test_a_device_that_goes_away_while_planning_is_reported_not_raised(
+        tmp_path, monkeypatch):
+    tracks = {1: _track(1, "Fine", tmp_path=tmp_path)}
+    monkeypatch.setattr(
+        "lintunes.andtunes.sync.plan_andtunes_sync",
+        lambda *a, **kw: (_ for _ in ()).throw(OSError("device went away")))
+    worker = AndTunesPlanWorker(
+        [_playlist(1)], tracks, _root(tmp_path), None, copy_art=False)
+    got = {}
+    worker.planned.connect(lambda plan, free: got.update(plan=plan))
+    worker.failed.connect(lambda m: got.update(failed=m))
+
+    worker._run()
+
+    assert "plan" not in got
+    assert "device went away" in got["failed"]