travandClaude Opus 5
2d961c0c9e
The merge windows kept coming because finishing a track rewrote all 15 MB of library.json. Both machines did that, so Syncthing saw two edits to one big file between syncs and produced a conflict file roughly once per song — and the merge then took max() of the two counts, discarding whichever side had played less. The .resolved/ backups showed the last nine library.json merges were ~98% play counts plus exactly one real edit, with the same ~45 tracks disagreeing every time and the count only creeping down over a day. library.json now holds only a base count. Each machine owns plays/<machine-id>.json with its own per-track totals, and the effective count is base + the sum of every journal. Only the owner writes its journal, so play data can't conflict; the journal stores totals rather than an append log, so there's no compaction step to double-count in; and a machine still on older code keeps bumping its own base, which stays additive with our journal. Two places carry the whole hazard, and both are pinned by tests: save_tracks writes journal.base_fields(), never the Track's effective count, and PlayJournal.load must be given a base-valued library — which is why reload_from_disk folds the journals onto the disk copy before reconciling. On the real 21k library: three plays write 83 bytes and leave library.json untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8Mzze7shr5pZoEgKQU5NW
312 lines
13 KiB
Python
312 lines
13 KiB
Python
"""Round 38: one merge window, and play counts that can't conflict.
|
|
|
|
Two bugs stacked fifteen "Synced changes merged" windows on the desktop: the
|
|
dialog was never reused, and playing a song rewrote all 15 MB of library.json —
|
|
so Syncthing produced a conflict file roughly once per track. The dialog is now
|
|
a session-long log, and play counts live in per-machine journal files that only
|
|
their owner ever writes.
|
|
"""
|
|
from datetime import datetime
|
|
|
|
import pytest
|
|
|
|
from lintunes.storage.conflict_resolver import ConflictSummary
|
|
|
|
|
|
def _summary(file="library.json", lines=("2 track(s) reconciled.",), backup="/b/1"):
|
|
return ConflictSummary(file, "library", list(lines), backup)
|
|
|
|
|
|
@pytest.fixture
|
|
def dialog_cls(qapp):
|
|
from lintunes.gui.conflict_dialog import ConflictSummaryDialog
|
|
return ConflictSummaryDialog
|
|
|
|
|
|
class TestOneMergeWindow:
|
|
def test_second_merge_appends_instead_of_replacing(self, dialog_cls):
|
|
dlg = dialog_cls([_summary(lines=["first merge"])], manager=None)
|
|
dlg.add_event([_summary(lines=["second merge"])])
|
|
|
|
text = dlg._body.toPlainText()
|
|
assert "first merge" in text and "second merge" in text
|
|
# Newest first.
|
|
assert text.index("second merge") < text.index("first merge")
|
|
|
|
def test_events_are_timestamped_once_there_is_more_than_one(self, dialog_cls):
|
|
dlg = dialog_cls([_summary()], manager=None)
|
|
assert "──" not in dlg._body.toPlainText() # a lone merge needs no header
|
|
|
|
dlg.add_event([_summary()], when=datetime(2026, 8, 20, 18, 42))
|
|
text = dlg._body.toPlainText()
|
|
assert "6:42 PM" in text
|
|
assert "2 merges this session." in dlg._intro.text()
|
|
|
|
def test_restore_targets_the_newest_backup_and_names_it(self, dialog_cls):
|
|
dlg = dialog_cls([_summary(backup="/b/old")], manager=None)
|
|
assert dlg._backup_dir == "/b/old"
|
|
|
|
dlg.add_event([_summary(backup="/b/new")], when=datetime(2026, 8, 20, 18, 42))
|
|
assert dlg._backup_dir == "/b/new"
|
|
assert dlg._restore_btn.text() == "Restore backup from 6:42 PM"
|
|
|
|
def test_empty_event_leaves_the_previous_backup_reachable(self, dialog_cls):
|
|
dlg = dialog_cls([_summary(backup="/b/old")], manager=None)
|
|
dlg.add_event([])
|
|
assert dlg._backup_dir == "/b/old"
|
|
assert dlg._restore_btn.isEnabled()
|
|
|
|
def test_no_backup_disables_the_backup_buttons(self, dialog_cls):
|
|
dlg = dialog_cls([_summary(backup="")], manager=None)
|
|
assert not dlg._open_btn.isEnabled()
|
|
assert not dlg._restore_btn.isEnabled()
|
|
|
|
|
|
class TestWindowReuse:
|
|
"""MainWindow isn't constructible headless, so exercise the two real methods
|
|
on a minimal QWidget host — that's where the stacking bug lived."""
|
|
|
|
@pytest.fixture
|
|
def host(self, qapp):
|
|
from PyQt6.QtWidgets import QWidget
|
|
from lintunes.gui.main_window import MainWindow
|
|
|
|
class Host(QWidget):
|
|
_manager = None
|
|
_conflict_dialog = None
|
|
show_conflict_summary = MainWindow.show_conflict_summary
|
|
_on_conflict_dialog_closed = MainWindow._on_conflict_dialog_closed
|
|
|
|
return Host()
|
|
|
|
def test_repeated_merges_make_exactly_one_dialog(self, host):
|
|
from lintunes.gui.conflict_dialog import ConflictSummaryDialog
|
|
|
|
for i in range(15):
|
|
host.show_conflict_summary([_summary(lines=[f"merge {i}"])])
|
|
|
|
dialogs = host.findChildren(ConflictSummaryDialog)
|
|
assert len(dialogs) == 1
|
|
text = dialogs[0]._body.toPlainText()
|
|
assert "merge 0" in text and "merge 14" in text
|
|
|
|
def test_closing_lets_the_next_merge_open_a_fresh_window(self, host):
|
|
host.show_conflict_summary([_summary()])
|
|
first = host._conflict_dialog
|
|
first.accept() # "Keep merged"
|
|
assert host._conflict_dialog is None
|
|
|
|
host.show_conflict_summary([_summary()])
|
|
assert host._conflict_dialog is not None
|
|
assert host._conflict_dialog is not first
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# Per-machine play journals
|
|
# --------------------------------------------------------------------------
|
|
|
|
from lintunes.models import Library, Track # noqa: E402
|
|
from lintunes.storage import json_storage # noqa: E402
|
|
from lintunes.storage.play_journal import PlayJournal # noqa: E402
|
|
|
|
|
|
@pytest.fixture
|
|
def data_dir(tmp_path, monkeypatch):
|
|
"""A data dir with a two-track library, and a machine id kept out of the
|
|
real ~/.config."""
|
|
monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "config"))
|
|
d = tmp_path / "data"
|
|
library = Library(tracks={
|
|
1: Track(track_id=1, name="Thunder Peel", play_count=102),
|
|
2: Track(track_id=2, name="Black Blood", play_count=42),
|
|
})
|
|
json_storage.save_library(library, d)
|
|
return d
|
|
|
|
|
|
def _base_on_disk(data_dir, tid=1, field="play_count"):
|
|
return json_storage.read_json(data_dir / "library.json")[str(tid)].get(field, 0)
|
|
|
|
|
|
def _write_journal(data_dir, machine, entries):
|
|
(data_dir / "plays").mkdir(parents=True, exist_ok=True)
|
|
json_storage.write_json(data_dir / "plays" / f"{machine}.json", entries)
|
|
|
|
|
|
class TestPlayJournal:
|
|
def test_machine_id_is_stable_and_outside_the_data_dir(self, data_dir, tmp_path):
|
|
from lintunes.storage.play_journal import machine_id
|
|
first = machine_id()
|
|
assert first and machine_id() == first
|
|
# A synced id would make both machines share one journal.
|
|
assert not (data_dir / "machine_id").exists()
|
|
assert (tmp_path / "config" / "lintunes" / "machine_id").exists()
|
|
|
|
def test_effective_count_is_base_plus_every_journal(self, data_dir):
|
|
_write_journal(data_dir, "machine-a", {"1": {"plays": 5, "last_played": "2026-08-20T10:00:00"}})
|
|
_write_journal(data_dir, "machine-b", {"1": {"plays": 3, "last_played": "2026-08-20T12:00:00"}})
|
|
|
|
library = json_storage.load_library(data_dir)
|
|
PlayJournal(machine="machine-a").load(data_dir, library)
|
|
|
|
# This is the case max() used to throw away: 102 + 5 + 3, not 107.
|
|
assert library.tracks[1].play_count == 110
|
|
assert library.tracks[1].play_date_utc == "2026-08-20T12:00:00"
|
|
assert library.tracks[2].play_count == 42 # untouched
|
|
|
|
def test_a_play_moves_the_journal_and_leaves_library_json_alone(self, data_dir):
|
|
library = json_storage.load_library(data_dir)
|
|
journal = PlayJournal(machine="machine-a")
|
|
journal.load(data_dir, library)
|
|
|
|
before = (data_dir / "library.json").read_bytes()
|
|
journal.record_play(1, "2026-08-20T18:42:00")
|
|
journal.save(data_dir)
|
|
|
|
assert (data_dir / "library.json").read_bytes() == before
|
|
assert json_storage.read_json(data_dir / "plays" / "machine-a.json") == {
|
|
"1": {"plays": 1, "last_played": "2026-08-20T18:42:00"}}
|
|
|
|
def test_save_tracks_writes_the_base_not_the_effective_count(self, data_dir):
|
|
"""The one sharp edge: writing the folded total back into library.json
|
|
would fold this machine's plays into the base and count them twice."""
|
|
_write_journal(data_dir, "machine-a", {"1": {"plays": 5}})
|
|
library = json_storage.load_library(data_dir)
|
|
journal = PlayJournal(machine="machine-a")
|
|
journal.load(data_dir, library)
|
|
assert library.tracks[1].play_count == 107
|
|
|
|
json_storage.save_tracks(library, data_dir, journal)
|
|
assert _base_on_disk(data_dir) == 102
|
|
|
|
# And a second round trip must not drift either.
|
|
again = json_storage.load_library(data_dir)
|
|
PlayJournal(machine="machine-a").load(data_dir, again)
|
|
assert again.tracks[1].play_count == 107
|
|
|
|
def test_zero_base_is_omitted_from_library_json(self, data_dir):
|
|
"""to_dict() omits default-valued fields; the base rewrite must too, or
|
|
21k tracks each grow a redundant "play_count": 0."""
|
|
library = json_storage.load_library(data_dir)
|
|
library.tracks[3] = Track(track_id=3, name="New Song")
|
|
journal = PlayJournal(machine="machine-a")
|
|
journal.load(data_dir, library)
|
|
journal.record_play(3, "2026-08-20T18:42:00")
|
|
|
|
json_storage.save_tracks(library, data_dir, journal)
|
|
assert "play_count" not in json_storage.read_json(
|
|
data_dir / "library.json")["3"]
|
|
|
|
def test_other_machine_on_old_code_does_not_double_count(self, data_dir):
|
|
"""While the other machine still bumps its base in library.json, our
|
|
journal stays additive on top — no double count, no loss."""
|
|
_write_journal(data_dir, "machine-a", {"1": {"plays": 5}})
|
|
library = json_storage.load_library(data_dir)
|
|
PlayJournal(machine="machine-a").load(data_dir, library)
|
|
assert library.tracks[1].play_count == 107
|
|
|
|
# The other machine, still on old code, syncs in a base bumped by 3.
|
|
raw = json_storage.read_json(data_dir / "library.json")
|
|
raw["1"]["play_count"] = 105
|
|
json_storage.write_json(data_dir / "library.json", raw)
|
|
|
|
library = json_storage.load_library(data_dir)
|
|
PlayJournal(machine="machine-a").load(data_dir, library)
|
|
assert library.tracks[1].play_count == 110
|
|
|
|
def test_unsaved_plays_survive_a_journal_reread(self, data_dir):
|
|
"""A reload re-reads every journal; ours is only ever written by us, so
|
|
in-memory bumps must not be read over."""
|
|
library = json_storage.load_library(data_dir)
|
|
journal = PlayJournal(machine="machine-a")
|
|
journal.load(data_dir, library)
|
|
journal.record_play(1, "2026-08-20T18:42:00")
|
|
|
|
fresh = json_storage.load_library(data_dir)
|
|
journal.load(data_dir, fresh) # what reload_from_disk does
|
|
assert fresh.tracks[1].play_count == 103
|
|
assert journal.dirty
|
|
|
|
|
|
class TestManagerRoundTrip:
|
|
"""End to end through LibraryManager, which is where a base/effective mix-up
|
|
would actually cost trav play counts."""
|
|
|
|
@pytest.fixture
|
|
def manager(self, data_dir, qapp):
|
|
from lintunes.library_manager import LibraryManager
|
|
return LibraryManager(json_storage.load_library(data_dir), data_dir)
|
|
|
|
def test_playing_does_not_rewrite_library_json(self, manager, data_dir):
|
|
before = (data_dir / "library.json").read_bytes()
|
|
manager.record_play(1)
|
|
manager.flush()
|
|
|
|
assert (data_dir / "library.json").read_bytes() == before
|
|
assert manager.library.tracks[1].play_count == 103
|
|
|
|
def test_a_play_survives_a_restart_exactly_once(self, manager, data_dir):
|
|
manager.record_play(1)
|
|
manager.flush()
|
|
|
|
from lintunes.library_manager import LibraryManager
|
|
restarted = LibraryManager(json_storage.load_library(data_dir), data_dir)
|
|
assert restarted.library.tracks[1].play_count == 103
|
|
assert _base_on_disk(data_dir) == 102
|
|
|
|
def test_an_edit_after_a_play_still_writes_the_base(self, manager, data_dir):
|
|
"""The dangerous sequence: a play inflates the in-memory count, then an
|
|
unrelated edit flushes the whole library.json."""
|
|
manager.record_play(1)
|
|
manager.add_track(Track(track_id=9, name="Sev Beni Beni"))
|
|
manager.flush()
|
|
|
|
assert _base_on_disk(data_dir) == 102
|
|
assert manager.library.tracks[1].play_count == 103
|
|
|
|
from lintunes.library_manager import LibraryManager
|
|
restarted = LibraryManager(json_storage.load_library(data_dir), data_dir)
|
|
assert restarted.library.tracks[1].play_count == 103
|
|
assert 9 in restarted.library.tracks
|
|
|
|
def test_reload_does_not_inflate_the_count(self, manager, data_dir):
|
|
"""reload_from_disk reconciles in-memory (effective) against disk (base);
|
|
folding the journals onto the disk copy first is what keeps max() honest."""
|
|
manager.record_play(1)
|
|
manager.flush()
|
|
manager.reload_from_disk()
|
|
assert manager.library.tracks[1].play_count == 103
|
|
|
|
manager.reload_from_disk()
|
|
manager.flush()
|
|
assert manager.library.tracks[1].play_count == 103
|
|
assert _base_on_disk(data_dir) == 102
|
|
|
|
def test_the_other_machines_plays_arrive_on_reload(self, manager, data_dir):
|
|
_write_journal(data_dir, "machine-other", {"1": {"plays": 7}})
|
|
manager.reload_from_disk()
|
|
assert manager.library.tracks[1].play_count == 109
|
|
|
|
|
|
class TestJournalConflictInsurance:
|
|
"""A journal has one writer, so it should never conflict. If one ever does,
|
|
falling through to newest-wins would discard a machine's whole history."""
|
|
|
|
def test_conflicting_journals_merge_by_highest_total(self, data_dir):
|
|
from lintunes.storage import conflict_resolver
|
|
_write_journal(data_dir, "machine-a", {"1": {"plays": 9, "last_played": "2026-08-20T10:00:00"},
|
|
"2": {"plays": 4}})
|
|
json_storage.write_json(
|
|
data_dir / "plays" / "machine-a.sync-conflict-20260820-184200-ABCDEFG.json",
|
|
{"1": {"plays": 3, "last_played": "2026-08-20T18:00:00"},
|
|
"3": {"plays": 6}})
|
|
|
|
summaries = conflict_resolver.resolve_conflicts(data_dir)
|
|
assert [s.kind for s in summaries] == ["plays"]
|
|
|
|
merged = json_storage.read_json(data_dir / "plays" / "machine-a.json")
|
|
assert merged["1"]["plays"] == 9 # not 3
|
|
assert merged["1"]["last_played"] == "2026-08-20T18:00:00"
|
|
assert merged["2"]["plays"] == 4 # kept
|
|
assert merged["3"]["plays"] == 6 # gained
|