diff --git a/termstory/backup.py b/termstory/backup.py index 5f08a1d..779b62e 100644 --- a/termstory/backup.py +++ b/termstory/backup.py @@ -46,8 +46,9 @@ def backup_db() -> str: oldest = backups.pop(0) if os.path.exists(oldest): os.remove(oldest) - except OSError as e: - logger.warning("Failed to rotate old backups: %s", e) + except OSError: + # Rotation failure should not crash the backup process, but it must be visible. + logger.exception("Failed to rotate old backups in %s", backup_dir) return backup_path diff --git a/tests/test_backup.py b/tests/test_backup.py index e67a11c..6bc18e9 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -1,3 +1,4 @@ +import logging import os import pytest from termstory.backup import backup_db, restore_db @@ -84,3 +85,46 @@ def now(cls): import glob remaining = glob.glob(os.path.join(backup_dir, "termstory_backup_*.db")) assert len(remaining) == 10 + + +def test_backup_survives_rotation_failure(tmp_path, monkeypatch, caplog): + db_file = tmp_path / "test_rotation_failure.db" + db_path = str(db_file) + monkeypatch.setenv("DB_PATH", db_path) + monkeypatch.setattr("termstory.config.get_db_path", lambda: db_path) + monkeypatch.setattr("termstory.backup.get_db_path", lambda: db_path) + + db = Database(db_path) + db.init_db() + + class MockDatetime: + counter = 0 + @classmethod + def now(cls): + cls.counter += 1 + from datetime import datetime as dt + return dt(2026, 6, 18, 19, 0, cls.counter) + + monkeypatch.setattr("termstory.backup.datetime", MockDatetime) + + # Fill past the rotation threshold so cleanup runs and hits the failing os.remove. + for _ in range(11): + backup_db() + + def boom(_path): + raise PermissionError("cannot delete backup") + + monkeypatch.setattr("termstory.backup.os.remove", boom) + + # Rotation now fails, but the backup itself must still succeed and return a path. + with caplog.at_level(logging.ERROR, logger="termstory.backup"): + backup_path = backup_db() + + assert os.path.isfile(backup_path) + + # The rotation failure must be logged, not silently swallowed. A regression + # back to `except OSError: pass` would drop this record and fail here. + assert any( + "Failed to rotate old backups" in record.message + for record in caplog.records + )