fix: Improve "Auto-remove" ("Remove & Retention") GUI and its documentation

common/config.py

@@ -929,26 +929,12 @@ def setKeepOnlyOneSnapshot(self, value, profile_id = None):
     def removeOldSnapshotsEnabled(self, profile_id = None):
         return self.profileBoolValue('snapshots.remove_old_snapshots.enabled', True, profile_id)
 
-    def removeOldSnapshotsDate(self, profile_id = None):
+    def removeOldSnapshotsDate(self, profile_id=None):
         enabled, value, unit = self.removeOldSnapshots(profile_id)
         if not enabled:
             return datetime.date(1, 1, 1)
 
-        if unit == self.DAY:
-            date = datetime.date.today()
-            date = date - datetime.timedelta(days = value)
-            return date
-
-        if unit == self.WEEK:
-            date = datetime.date.today()
-            date = date - datetime.timedelta(days = date.weekday() + 7 * value)
-            return date
-
-        if unit == self.YEAR:
-            date = datetime.date.today()
-            return date.replace(day = 1, year = date.year - value)
-
-        return datetime.date(1, 1, 1)
+        return _remove_old_snapshots_date(value, unit)
 
     def setRemoveOldSnapshots(self, enabled, value, unit, profile_id = None):
         self.setProfileBoolValue('snapshots.remove_old_snapshots.enabled', enabled, profile_id)
@@ -1644,3 +1630,27 @@ def _cron_cmd(self, profile_id):
             cmd = tools.which('nice') + ' -n19 ' + cmd
 
         return cmd
+
+
+def _remove_old_snapshots_date(value, unit):
+    """Dev note (buhtz, 2025-01): The function exist to decople that code from
+    Config class and make it testable to investigate its behavior.
+
+    See issue #1943 for further reading.
+    """
+    if unit == Config.DAY:
+        date = datetime.date.today()
+        date = date - datetime.timedelta(days=value)
+        return date
+
+    if unit == Config.WEEK:
+        date = datetime.date.today()
+        # Always beginning (Monday) of the week
+        date = date - datetime.timedelta(days=date.weekday() + 7 * value)
+        return date
+
+    if unit == Config.YEAR:
+        date = datetime.date.today()
+        return date.replace(day=1, year=date.year - value)
+
+    return datetime.date(1, 1, 1)

common/snapshots.py

@@ -1860,7 +1860,6 @@ def freeSpace(self, now):
         """
         snapshots = listSnapshots(self.config, reverse=False)
         if not snapshots:
-            logger.debug('No snapshots. Skip freeSpace', self)
             return
 
         last_snapshot = snapshots[-1]
@@ -1870,7 +1869,8 @@ def freeSpace(self, now):
             self.setTakeSnapshotMessage(0, _('Removing old snapshots'))
 
             oldBackupId = SID(self.config.removeOldSnapshotsDate(), self.config)
-            logger.debug("Remove snapshots older than: {}".format(oldBackupId.withoutTag), self)
+            logger.debug("Remove snapshots older than: {}"
+                         .format(oldBackupId.withoutTag), self)
 
             while True:
                 if len(snapshots) <= 1:

common/test/test_config.py

@@ -1,4 +1,5 @@
 # SPDX-FileCopyrightText: © 2016 Taylor Raack
+# SPDX-FileCopyrightText: © 2025 Christian Buhtz <[email protected]>
 #
 # SPDX-License-Identifier: GPL-2.0-or-later
 #
@@ -10,11 +11,71 @@
 import os
 import sys
 import getpass
+import unittest
+import datetime
+from unittest.mock import patch
 from test import generic
 sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
+import config
 
 
-class TestSshCommand(generic.SSHTestCase):
+class RemoveOldSnapshotsDate(unittest.TestCase):
+    # pylint: disable=protected-access
+    def test_invalid_unit(self):
+        """1st January Year 1 on errors"""
+        unit = 99999
+        value = 99
+
+        self.assertEqual(
+            config._remove_old_snapshots_date(value, unit),
+            datetime.date(1, 1, 1))
+
+    @patch('datetime.date', wraps=datetime.date)
+    def test_day(self, m):
+        """Three days"""
+        m.today.return_value = datetime.date(2025, 1, 10)
+        sut = config._remove_old_snapshots_date(3, config.Config.DAY)
+        self.assertEqual(sut, datetime.date(2025, 1, 7))
+
+    @patch('datetime.date', wraps=datetime.date)
+    def test_week_always_monday(self, m):
+        """Result is always a Monday"""
+
+        # 1-53 weeks back
+        for weeks in range(1, 54):
+            start = datetime.date(2026, 1, 1)
+
+            # Every day in the year
+            for count in range(366):
+                m.today.return_value = start - datetime.timedelta(days=count)
+
+                sut = config._remove_old_snapshots_date(
+                    weeks, config.Config.WEEK)
+
+                # 0=Monday
+                self.assertEqual(sut.weekday(), 0, f'{sut=} {weeks=}')
+
+    @patch('datetime.date', wraps=datetime.date)
+    def test_week_ignore_current(self, m):
+        """Current (incomplete) week is ignored."""
+        for day in range(25, 32):  # Monday (25th) to Sunday (31th)
+            m.today.return_value = datetime.date(2025, 8, day)
+            sut = config._remove_old_snapshots_date(2, config.Config.WEEK)
+            self.assertEqual(
+                sut,
+                datetime.date(2025, 8, 11)  # Monday
+            )
+
+    @patch('datetime.date', wraps=datetime.date)
+    def test_year_ignore_current_month(self, m):
+        """Not years but 12 months are counted. But current month is
+        ignored."""
+        m.today.return_value = datetime.date(2025, 7, 30)
+        sut = config._remove_old_snapshots_date(2, config.Config.YEAR)
+        self.assertEqual(sut, datetime.date(2023, 7, 1))
+
+
+class SshCommand(generic.SSHTestCase):
     @classmethod
     def setUpClass(cls):
         cls._user = getpass.getuser()