upgrades: Minor refactor to pre-dist upgrade checks

- Don't perform sources.list changes in the check() method.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/upgrades/distupgrade.py

@@ -82,7 +82,7 @@ def _get_new_codename(test_upgrade: bool) -> str | None:
     return None
 
 
-def check(test_upgrade=False):
+def _check(test_upgrade: bool = False) -> tuple[str, str]:
     """Check if a distribution upgrade be performed.
 
     Check for new stable release, if updates are enabled, and if there is
@@ -103,18 +103,18 @@ def check(test_upgrade=False):
         raise RuntimeError('found-previous')
 
     from plinth.modules.upgrades import get_current_release
-    release, dist = get_current_release()
+    release, old_codename = get_current_release()
     if release in ['unstable', 'testing', 'n/a']:
         raise RuntimeError(f'already-{release}')
 
-    codename = _get_new_codename(test_upgrade)
+    new_codename = _get_new_codename(test_upgrade)
-    if not codename:
+    if not new_codename:
         raise RuntimeError('codename-not-found')
 
-    if codename == dist:
+    if new_codename == old_codename:
-        raise RuntimeError(f'already-{dist}')
+        raise RuntimeError(f'already-{old_codename}')
 
-    _sources_list_update(dist, codename)
+    return old_codename, new_codename
 
 
 @contextlib.contextmanager
@@ -289,8 +289,12 @@ def perform():
     _apt_update()
 
 
-def start_service():
+def start_service(test_upgrade: bool):
     """Create dist upgrade service and start it."""
+    old_codename, new_codename = _check(test_upgrade)
+
+    _sources_list_update(old_codename, new_codename)
+
     old_service_path = pathlib.Path(
         '/run/systemd/system/freedombox-dist-upgrade.service')
     if old_service_path.exists():

plinth/modules/upgrades/privileged.py

@@ -244,8 +244,7 @@ def start_dist_upgrade(test: bool = False):
     """
     _release_held_freedombox()
 
-    distupgrade.check(test)
+    distupgrade.start_service(test)
-    distupgrade.start_service()
 
 
 @privileged

plinth/modules/upgrades/tests/test_distupgrade.py

@@ -99,46 +99,49 @@ Description: Debian Testing distribution
     assert not distupgrade._get_new_codename(True)
 
 
-@patch('plinth.modules.upgrades.distupgrade._sources_list_update')
 @patch('plinth.modules.upgrades.distupgrade._get_new_codename')
 @patch('plinth.modules.upgrades.get_current_release')
 @patch('plinth.action_utils.service_is_running')
 @patch('plinth.modules.upgrades.utils.is_sufficient_free_space')
 @patch('plinth.modules.upgrades.utils.check_auto')
 def test_check(check_auto, is_sufficient_free_space, service_is_running,
-               get_current_release, get_new_codename, sources_list_update):
+               get_current_release, get_new_codename):
     """Test checking for available dist upgrade."""
     check_auto.return_value = False
     with pytest.raises(RuntimeError, match='upgrades-not-enabled'):
-        distupgrade.check()
+        distupgrade._check()
 
     check_auto.return_value = True
     is_sufficient_free_space.return_value = False
     with pytest.raises(RuntimeError, match='not-enough-free-space'):
-        distupgrade.check()
+        distupgrade._check()
 
     is_sufficient_free_space.return_value = True
     service_is_running.return_value = True
     with pytest.raises(RuntimeError, match='found-previous'):
-        distupgrade.check()
+        distupgrade._check()
 
     service_is_running.return_value = False
     for release in ['unstable', 'testing', 'n/a']:
         get_current_release.return_value = (release, release)
         with pytest.raises(RuntimeError, match=f'already-{release}'):
-            distupgrade.check()
+            distupgrade._check()
 
     get_current_release.return_value = ('12', 'bookworm')
     get_new_codename.return_value = None
     with pytest.raises(RuntimeError, match='codename-not-found'):
-        distupgrade.check()
+        distupgrade._check()
+        get_new_codename.assert_called_with(False)
+
+        distupgrade._check(True)
+        get_new_codename.assert_called_with(True)
 
     get_new_codename.return_value = 'bookworm'
     with pytest.raises(RuntimeError, match='already-bookworm'):
-        distupgrade.check()
+        distupgrade._check()
 
     get_new_codename.return_value = 'trixie'
-    sources_list_update.call_args_list = [call('bookworm', 'trixie')]
+    assert distupgrade._check() == ('bookworm', 'trixie')
 
 
 @patch('subprocess.run')