travandClaude Opus 5
7df5ecf868
Restarting on the machine that mounts the drive at /media/trav/muzak opened "LinTunes can't find your music folder", offering ~/Music. The library's only music_folder is the other machine's mount point (/run/media/trav/tummult/...) because this library predates 0.10 and has no music_folder_rel yet — but config.json has held the right path all along under music_root, which `--music-root … --save-config` writes and the importer stores as library.music_folder. resolve() only looked for music_folder_override, a key that exists solely because the prompt writes it, so a library imported the normal way could never satisfy the check that decides whether to prompt. music_root is now a machine-local candidate alongside music_folder_override, consulted in the same last-resort position and subject to the same must-exist rule, so a stale one still reports "missing" rather than resolving to a phantom tree. Read-only: nothing writes it back, and the newer key still wins when both are present. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9ZEFi4qNJ39FMiBtiAxy2
512 lines
21 KiB
Python
512 lines
21 KiB
Python
"""Round 40 — welcoming the newcomers.
|
|
|
|
Three defects, all of which hit a *new* user hardest:
|
|
|
|
* Startup could open a blocking font dialog before the main window existed, so
|
|
the very first thing LinTunes did on a fresh machine was demand a decision
|
|
about Century Gothic. Now the best available geometric sans is chosen
|
|
silently and the font lives in Preferences.
|
|
* A non-iTunes user had no supported way to set their music folder at all, and
|
|
with it unset imports fell back to a *relative* ``Path("Music")`` — resolved
|
|
against the working directory, which GNOME's dash does not set predictably.
|
|
Music scattered somewhere unfindable.
|
|
* ``music_folder`` was the only path in the library stored raw absolute in the
|
|
*synced* metadata, so a second machine inherited the first machine's paths.
|
|
It is now stored relative as well (both spellings, so an older LinTunes on
|
|
the other machine keeps working), with a semantic stamp so a merge picks the
|
|
newest *setting* rather than the newest *file*.
|
|
"""
|
|
|
|
import json
|
|
|
|
import pytest
|
|
from PyQt6.QtCore import QObject, pyqtSignal
|
|
|
|
from lintunes import music_folder, theme
|
|
from lintunes.library_manager import LibraryManager
|
|
from lintunes.models import Library, Track
|
|
from lintunes.preferences import Preferences
|
|
from lintunes.storage import json_storage
|
|
from lintunes.storage.conflict_resolver import resolve_conflicts
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# helpers
|
|
|
|
|
|
class _FakeLastFm(QObject):
|
|
login_finished = pyqtSignal(bool, str)
|
|
status_message = pyqtSignal(str)
|
|
|
|
def is_logged_in(self):
|
|
return False
|
|
|
|
|
|
class _FakePrefs:
|
|
"""Just enough of Preferences for the pure font resolution."""
|
|
|
|
def __init__(self, value=None):
|
|
self._value = value
|
|
|
|
def get(self, key, default=None):
|
|
return self._value if key == "now_playing_font" else default
|
|
|
|
|
|
def _families(monkeypatch, *names):
|
|
"""Pretend this machine has exactly these font families."""
|
|
class _DB:
|
|
@staticmethod
|
|
def families():
|
|
return list(names)
|
|
monkeypatch.setattr(theme, "QFontDatabase", _DB)
|
|
|
|
|
|
@pytest.fixture
|
|
def isolated_config(tmp_path, monkeypatch):
|
|
"""Point config.json at a temp dir so tests never touch the real one."""
|
|
monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config"))
|
|
return tmp_path / "config"
|
|
|
|
|
|
def _library(tmp_path, count=2, music=None):
|
|
music = music or (tmp_path / "media")
|
|
library = Library(music_folder=str(music))
|
|
for tid in range(1, count + 1):
|
|
path = music / "Music" / f"Artist{tid}" / "Album" / f"T{tid}.mp3"
|
|
path.parent.mkdir(parents=True, exist_ok=True)
|
|
path.write_bytes(b"audio")
|
|
library.tracks[tid] = Track(track_id=tid, name=f"T{tid}",
|
|
artist=f"Artist{tid}", location=str(path))
|
|
return library
|
|
|
|
|
|
def _manager(tmp_path, library):
|
|
return LibraryManager(library, tmp_path / "data")
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# A. the font no longer stops the app
|
|
|
|
|
|
class TestNowPlayingFont:
|
|
def test_prefers_century_gothic_when_installed(self, monkeypatch):
|
|
_families(monkeypatch, "DejaVu Sans", "Century Gothic", "URW Gothic")
|
|
assert theme.now_playing_font_family(_FakePrefs(None)) == "Century Gothic"
|
|
|
|
def test_falls_back_to_the_closest_geometric_sans(self, monkeypatch):
|
|
_families(monkeypatch, "DejaVu Sans", "URW Gothic", "Montserrat")
|
|
# URW Gothic is the Avant Garde clone Century Gothic derives from, and
|
|
# sits ahead of Montserrat in the chain.
|
|
assert theme.now_playing_font_family(_FakePrefs(None)) == "URW Gothic"
|
|
|
|
def test_returns_none_when_nothing_comparable_exists(self, monkeypatch):
|
|
_families(monkeypatch, "DejaVu Sans", "Liberation Serif")
|
|
assert theme.now_playing_font_family(_FakePrefs(None)) is None
|
|
|
|
def test_empty_string_always_means_the_app_font(self, monkeypatch):
|
|
_families(monkeypatch, "Century Gothic")
|
|
assert theme.now_playing_font_family(_FakePrefs("")) is None
|
|
|
|
def test_a_chosen_family_wins_while_it_is_installed(self, monkeypatch):
|
|
_families(monkeypatch, "Century Gothic", "Comic Neue")
|
|
assert theme.now_playing_font_family(_FakePrefs("Comic Neue")) == "Comic Neue"
|
|
|
|
def test_uninstalled_choice_falls_back_to_auto_not_the_app_font(self, monkeypatch):
|
|
"""The behavior change: an uninstalled saved family used to drop to the
|
|
app default (and then nag on next launch). Now it quietly becomes the
|
|
nearest thing installed."""
|
|
_families(monkeypatch, "URW Gothic", "DejaVu Sans")
|
|
assert theme.now_playing_font_family(_FakePrefs("Gone Sans")) == "URW Gothic"
|
|
|
|
def test_startup_never_opens_a_font_dialog(self):
|
|
import inspect
|
|
from lintunes import main as main_module
|
|
source = inspect.getsource(main_module)
|
|
assert "QFontDialog" not in source
|
|
assert not hasattr(main_module, "_ensure_now_playing_font")
|
|
|
|
|
|
class TestFontPreferenceRow:
|
|
def _dialog(self, tmp_path, qapp):
|
|
from lintunes.gui.preferences_dialog import PreferencesDialog
|
|
return PreferencesDialog(Preferences(tmp_path), _FakeLastFm())
|
|
|
|
def test_building_the_dialog_writes_no_preference(self, qapp, tmp_path):
|
|
prefs = Preferences(tmp_path)
|
|
from lintunes.gui.preferences_dialog import PreferencesDialog
|
|
PreferencesDialog(prefs, _FakeLastFm())
|
|
assert not (tmp_path / "preferences.json").exists()
|
|
|
|
def test_the_three_modes_write_the_three_states(self, qapp, tmp_path):
|
|
from lintunes.gui.preferences_dialog import PreferencesDialog
|
|
prefs = Preferences(tmp_path)
|
|
dialog = PreferencesDialog(prefs, _FakeLastFm())
|
|
|
|
dialog._font_default.setChecked(True)
|
|
assert prefs.get("now_playing_font") == ""
|
|
assert not dialog._font_combo.isEnabled()
|
|
|
|
dialog._font_pick.setChecked(True)
|
|
assert prefs.get("now_playing_font") # an explicit family
|
|
assert dialog._font_combo.isEnabled()
|
|
|
|
dialog._font_auto.setChecked(True)
|
|
assert prefs.get("now_playing_font") is None # automatic
|
|
|
|
def test_dialog_still_builds_without_the_music_callbacks(self, qapp, tmp_path):
|
|
"""Back-compat: the older two-arg construction must keep working."""
|
|
dialog = self._dialog(tmp_path, qapp)
|
|
assert not hasattr(dialog, "_music_edit")
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# B. the music folder: portable, settable, and never guessed
|
|
|
|
|
|
class TestMusicFolderStorage:
|
|
def test_metadata_carries_both_spellings_and_a_stamp(self, tmp_path):
|
|
data_dir = tmp_path / "data"
|
|
library = _library(tmp_path)
|
|
library.music_folder_set_at = "2026-08-22T00:00:00"
|
|
json_storage.save_library(library, data_dir)
|
|
|
|
written = json.loads((data_dir / "library_metadata.json").read_text())
|
|
assert written["music_folder"] == str(tmp_path / "media") # legacy
|
|
assert written["music_folder_rel"] == "../media" # portable
|
|
assert written["music_folder_set_at"] == "2026-08-22T00:00:00"
|
|
|
|
def test_a_library_without_the_new_keys_still_loads(self, tmp_path, isolated_config):
|
|
"""Protects the live library: pre-0.10 metadata has only the absolute key."""
|
|
data_dir = tmp_path / "data"
|
|
data_dir.mkdir()
|
|
media = tmp_path / "media"
|
|
media.mkdir()
|
|
(data_dir / "library_metadata.json").write_text(
|
|
json.dumps({"music_folder": str(media)}))
|
|
|
|
loaded = json_storage.load_library(data_dir)
|
|
state = music_folder.resolve(loaded, data_dir)
|
|
assert state.path == media
|
|
assert state.source == "legacy"
|
|
|
|
def test_the_relative_key_survives_the_whole_tree_moving(self, tmp_path,
|
|
isolated_config):
|
|
"""The cross-machine case — the entire point of the change."""
|
|
import shutil
|
|
first = tmp_path / "machine1"
|
|
(first / "media").mkdir(parents=True)
|
|
library = Library(music_folder=str(first / "media"))
|
|
json_storage.save_library(library, first / "data")
|
|
|
|
second = tmp_path / "machine2"
|
|
shutil.move(str(first), str(second))
|
|
|
|
loaded = json_storage.load_library(second / "data")
|
|
state = music_folder.resolve(loaded, second / "data")
|
|
assert state.path == second / "media"
|
|
assert state.source == "library"
|
|
# ...even though the absolute key still names the old machine's path.
|
|
assert loaded.music_folder == str(first / "media")
|
|
|
|
def test_old_code_can_still_read_an_absolute_path(self, tmp_path, isolated_config):
|
|
"""Forward-compat pin: the legacy key stays usable for the other
|
|
machine while it is still on older code."""
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
media = tmp_path / "elsewhere"
|
|
media.mkdir()
|
|
manager.set_music_folder(media)
|
|
|
|
written = json.loads(
|
|
(tmp_path / "data" / "library_metadata.json").read_text())
|
|
assert written["music_folder"] == str(media)
|
|
assert written["music_folder"].startswith("/")
|
|
|
|
|
|
class TestMusicFolderResolution:
|
|
def test_the_override_is_only_used_when_the_stored_folder_is_gone(
|
|
self, tmp_path, isolated_config):
|
|
media = tmp_path / "media"
|
|
media.mkdir()
|
|
elsewhere = tmp_path / "elsewhere"
|
|
elsewhere.mkdir()
|
|
library = Library(music_folder=str(media))
|
|
music_folder.set_machine_override(str(elsewhere))
|
|
|
|
# Stored folder exists → it wins, override ignored.
|
|
assert music_folder.resolve(library, tmp_path).path == media
|
|
|
|
# Stored folder gone → the override is the escape hatch.
|
|
gone = Library(music_folder=str(tmp_path / "unplugged"))
|
|
state = music_folder.resolve(gone, tmp_path)
|
|
assert state.path == elsewhere
|
|
assert state.source == "machine"
|
|
|
|
def test_the_pre_010_music_root_key_counts_as_an_override(
|
|
self, tmp_path, isolated_config):
|
|
"""`--music-root … --save-config` has always written this machine's
|
|
media folder to config.json as `music_root`. Every library imported that
|
|
way already knows where its music is here, so a machine that mounts the
|
|
drive at a different path must not be asked to go find it."""
|
|
from lintunes.config import save_config
|
|
media = tmp_path / "media"
|
|
media.mkdir()
|
|
save_config({"music_root": str(media)})
|
|
|
|
# The shared value is another machine's mount point, unreachable here.
|
|
gone = Library(music_folder="/run/media/trav/tummult/music/iTunes Media")
|
|
state = music_folder.resolve(gone, tmp_path)
|
|
|
|
assert state.path == media
|
|
assert state.source == "machine"
|
|
|
|
def test_the_newer_override_key_still_wins(self, tmp_path, isolated_config):
|
|
from lintunes.config import save_config
|
|
chosen, imported = tmp_path / "chosen", tmp_path / "imported"
|
|
chosen.mkdir()
|
|
imported.mkdir()
|
|
save_config({"music_folder_override": str(chosen),
|
|
"music_root": str(imported)})
|
|
|
|
state = music_folder.resolve(Library(music_folder="/nope"), tmp_path)
|
|
|
|
assert state.path == chosen
|
|
|
|
def test_a_stale_music_root_does_not_resolve(self, tmp_path, isolated_config):
|
|
"""It's still a candidate, not an answer — the folder has to be there."""
|
|
from lintunes.config import save_config
|
|
save_config({"music_root": str(tmp_path / "unplugged")})
|
|
|
|
state = music_folder.resolve(Library(music_folder="/nope"), tmp_path)
|
|
|
|
assert state.path is None
|
|
assert state.source == "missing"
|
|
|
|
def test_unset_and_missing_are_different_conversations(self, tmp_path,
|
|
isolated_config):
|
|
assert music_folder.resolve(Library(), tmp_path).source == "unset"
|
|
state = music_folder.resolve(
|
|
Library(music_folder=str(tmp_path / "nope")), tmp_path)
|
|
assert state.source == "missing"
|
|
assert state.stored == str(tmp_path / "nope")
|
|
|
|
def test_a_folder_called_music_is_the_tree_not_its_parent(self, tmp_path):
|
|
"""Otherwise picking ~/Music files everything into ~/Music/Music."""
|
|
home_music = tmp_path / "Music"
|
|
home_music.mkdir()
|
|
assert music_folder.normalize_media_folder(home_music) == tmp_path
|
|
assert music_folder.organize_root_for(
|
|
music_folder.normalize_media_folder(home_music)) == home_music
|
|
|
|
def test_an_itunes_media_folder_is_stored_unchanged(self, tmp_path):
|
|
media = tmp_path / "iTunes Media"
|
|
(media / "Music").mkdir(parents=True)
|
|
assert music_folder.normalize_media_folder(media) == media
|
|
|
|
def test_a_music_folder_that_already_has_a_music_child_is_left_alone(self, tmp_path):
|
|
nested = tmp_path / "Music"
|
|
(nested / "Music").mkdir(parents=True)
|
|
assert music_folder.normalize_media_folder(nested) == nested
|
|
|
|
|
|
class TestSettingTheMusicFolder:
|
|
def test_setting_it_persists_immediately(self, qapp, tmp_path, isolated_config):
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
media = tmp_path / "new-media"
|
|
media.mkdir()
|
|
manager.set_music_folder(media)
|
|
|
|
assert manager._dirty_metadata is False # flushed, not left pending
|
|
on_disk = json.loads(
|
|
(tmp_path / "data" / "library_metadata.json").read_text())
|
|
assert on_disk["music_folder"] == str(media)
|
|
assert manager.organize_root() == media / "Music"
|
|
|
|
def test_a_machine_local_choice_never_touches_the_shared_library(
|
|
self, qapp, tmp_path, isolated_config):
|
|
"""The 'never clobber the other machine' pin.
|
|
|
|
This is the real scenario: the shared folder is a path that exists on
|
|
the *other* machine but not here, so this machine records its own
|
|
answer and leaves the synced metadata completely alone.
|
|
"""
|
|
library = Library(music_folder=str(tmp_path / "other-machines-drive"))
|
|
manager = _manager(tmp_path, library)
|
|
manager.mark_library_settings_dirty()
|
|
manager.flush()
|
|
metadata = tmp_path / "data" / "library_metadata.json"
|
|
before = metadata.read_text()
|
|
assert manager.organize_root() is None # not reachable here
|
|
|
|
here = tmp_path / "local-media"
|
|
here.mkdir()
|
|
manager.set_music_folder(here, machine_only=True)
|
|
|
|
assert metadata.read_text() == before # byte-identical
|
|
assert manager.organize_root() == here / "Music"
|
|
assert music_folder.machine_override() == str(here)
|
|
|
|
def test_organize_root_is_none_when_the_folder_is_not_here(
|
|
self, qapp, tmp_path, isolated_config):
|
|
library = Library(music_folder=str(tmp_path / "unplugged-drive"))
|
|
manager = _manager(tmp_path, library)
|
|
assert manager.organize_root() is None
|
|
|
|
|
|
class TestSyncDoesNotRevertTheSetting:
|
|
def _write_metadata(self, data_dir, folder, stamp):
|
|
data_dir.mkdir(parents=True, exist_ok=True)
|
|
(data_dir / "library_metadata.json").write_text(json.dumps({
|
|
"music_folder": str(folder),
|
|
"music_folder_rel": "",
|
|
"music_folder_set_at": stamp,
|
|
"library_settings": {},
|
|
}))
|
|
|
|
def test_a_stale_synced_file_does_not_revert_a_newer_local_change(
|
|
self, qapp, tmp_path, isolated_config):
|
|
"""The trap: the dirty flag only guards until flush. After that an
|
|
older library_metadata.json arriving from the other machine would
|
|
silently undo a deliberate change."""
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
mine = tmp_path / "mine"
|
|
mine.mkdir()
|
|
manager.set_music_folder(mine) # flushes; now clean
|
|
|
|
theirs = tmp_path / "theirs"
|
|
theirs.mkdir()
|
|
self._write_metadata(tmp_path / "data", theirs, "2000-01-01T00:00:00")
|
|
manager.reload_from_disk()
|
|
|
|
assert manager.organize_root() == mine / "Music"
|
|
|
|
def test_a_newer_remote_change_is_still_adopted(self, qapp, tmp_path,
|
|
isolated_config):
|
|
"""The inverse — the guard must not lock the setting forever."""
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
mine = tmp_path / "mine"
|
|
mine.mkdir()
|
|
manager.set_music_folder(mine)
|
|
|
|
theirs = tmp_path / "theirs"
|
|
theirs.mkdir()
|
|
self._write_metadata(tmp_path / "data", theirs, "2099-01-01T00:00:00")
|
|
manager.reload_from_disk()
|
|
|
|
assert manager.organize_root() == theirs / "Music"
|
|
|
|
def test_a_legacy_library_still_trusts_disk(self, qapp, tmp_path,
|
|
isolated_config):
|
|
"""No stamps on either side → the pre-0.10 behavior, unchanged."""
|
|
manager = _manager(tmp_path, _library(tmp_path))
|
|
theirs = tmp_path / "theirs"
|
|
theirs.mkdir()
|
|
self._write_metadata(tmp_path / "data", theirs, None)
|
|
manager.reload_from_disk()
|
|
assert manager.organize_root() == theirs / "Music"
|
|
|
|
def test_a_conflict_keeps_the_most_recently_set_folder(self, tmp_path):
|
|
"""_merge_metadata picks the whole file by mtime, which moves for
|
|
cosmetic reasons; the folder must be decided on its own stamp."""
|
|
import os
|
|
import time
|
|
data_dir = tmp_path / "data"
|
|
data_dir.mkdir()
|
|
original = data_dir / "library_metadata.json"
|
|
conflict = data_dir / "library_metadata.sync-conflict-20260822-120000-ABCDEFG.json"
|
|
|
|
# The conflict copy set the folder later, but the local file was
|
|
# touched more recently (someone resized a column).
|
|
conflict.write_text(json.dumps({
|
|
"music_folder": "/other/media",
|
|
"music_folder_rel": "../other",
|
|
"music_folder_set_at": "2099-01-01T00:00:00",
|
|
}))
|
|
original.write_text(json.dumps({
|
|
"music_folder": "/old/media",
|
|
"music_folder_rel": "../old",
|
|
"music_folder_set_at": "2000-01-01T00:00:00",
|
|
}))
|
|
now = time.time()
|
|
os.utime(conflict, (now - 500, now - 500))
|
|
os.utime(original, (now, now))
|
|
|
|
resolve_conflicts(data_dir)
|
|
|
|
merged = json.loads(original.read_text())
|
|
assert merged["music_folder"] == "/other/media"
|
|
assert merged["music_folder_rel"] == "../other"
|
|
|
|
|
|
class TestImportsNeverScatter:
|
|
def test_no_relative_music_fallback_remains(self):
|
|
"""The scatter bug: `or Path("Music")` resolved against the working
|
|
directory, which GNOME's dash does not set predictably."""
|
|
import inspect
|
|
from lintunes.gui.main_window import MainWindow
|
|
source = inspect.getsource(MainWindow._music_import_dir)
|
|
code = [line for line in source.splitlines()
|
|
if line.strip().startswith("return")]
|
|
assert code == [" return self._manager.organize_root()"]
|
|
|
|
def test_import_refuses_rather_than_guessing(self, qapp, tmp_path,
|
|
isolated_config, monkeypatch):
|
|
from lintunes.gui.main_window import MainWindow
|
|
library = Library() # no music folder at all
|
|
manager = _manager(tmp_path, library)
|
|
window = MainWindow(manager, Preferences(tmp_path / "data"))
|
|
try:
|
|
# Pretend the user declines the prompt.
|
|
monkeypatch.setattr(window, "check_music_folder", lambda: None)
|
|
source = tmp_path / "song.mp3"
|
|
source.write_bytes(b"audio")
|
|
|
|
cwd_before = set(p.name for p in tmp_path.iterdir())
|
|
assert window.import_files([source]) == []
|
|
# Nothing created, nothing copied anywhere.
|
|
assert set(p.name for p in tmp_path.iterdir()) == cwd_before
|
|
finally:
|
|
window.close()
|
|
|
|
def test_files_land_under_the_music_folder_when_set(self, qapp, tmp_path,
|
|
isolated_config,
|
|
mp3_file):
|
|
from lintunes.gui.main_window import MainWindow
|
|
media = tmp_path / "media"
|
|
media.mkdir()
|
|
manager = _manager(tmp_path, Library(music_folder=str(media)))
|
|
window = MainWindow(manager, Preferences(tmp_path / "data"))
|
|
try:
|
|
window.import_files([mp3_file])
|
|
copied = list((media / "Music").rglob("*.mp3"))
|
|
assert len(copied) == 1
|
|
finally:
|
|
window.close()
|
|
|
|
|
|
class TestFirstRunPrompt:
|
|
def test_the_machine_is_asked_only_once(self, tmp_path, isolated_config):
|
|
assert music_folder.prompt_seen() is False
|
|
music_folder.mark_prompt_seen()
|
|
assert music_folder.prompt_seen() is True
|
|
|
|
def test_the_flag_is_per_machine_not_synced(self, tmp_path, isolated_config):
|
|
"""It lives in config.json, not the synced preferences.json — each
|
|
machine may genuinely need its own answer."""
|
|
music_folder.mark_prompt_seen()
|
|
config = json.loads(
|
|
(isolated_config / "lintunes" / "config.json").read_text())
|
|
assert config[music_folder.PROMPT_SEEN_KEY] is True
|
|
|
|
def test_the_dialog_shows_where_songs_will_actually_go(self, qapp, tmp_path):
|
|
from lintunes.gui.music_folder_dialog import MusicFolderDialog
|
|
dialog = MusicFolderDialog()
|
|
try:
|
|
target = tmp_path / "Tunes"
|
|
target.mkdir()
|
|
dialog._folder.setText(str(target))
|
|
dialog._refresh_preview()
|
|
assert str(target / "Music" / "Artist" / "Album") in \
|
|
dialog._preview.text()
|
|
finally:
|
|
dialog.close()
|