updater: update tests and code refformating

Author: piotrbartman

qui/updater/intro_page.py

@@ -211,14 +211,15 @@ def select_rows(self):
         for row in self.list_store:
             row.selected = row.updates_available in self.head_checkbox.allowed
 
-    def select_rows_ignoring_conditions(self, cliargs, dom0):
+    def select_rows_ignoring_conditions(self, cliargs):
         cmd = ["qubes-vm-update", "--dry-run", "--quiet"]
 
         args = [a for a in dir(cliargs) if not a.startswith("_")]
         for arg in args:
             if arg in (
                 "non_interactive",
                 "non_default_select",
+                "dom0",
                 "restart",
                 "apply_to_sys",
                 "apply_to_all",
@@ -235,10 +236,12 @@ def select_rows_ignoring_conditions(self, cliargs, dom0):
                 cmd.append(f"--{arg.replace('_', '-')}")
                 if not isinstance(value, bool):
                     cmd.append(str(value))
+        if cliargs.dom0:
+            cmd.extend(["--targets", "dom0"])
 
         to_update = set()
         non_default_select = [
-            "--" + arg for arg in cliargs.non_default_select if arg != "dom0"  # TODO remove dom0 flag
+            "--" + arg for arg in cliargs.non_default_select if arg != "dom0"
         ]
         non_default = [a for a in cmd if a in non_default_select]
         if non_default or cliargs.non_interactive:
@@ -257,15 +260,19 @@ def _get_stale_qubes(self, cmd):
             result = set()
             if "dom0" in output_lines[0]:
                 result.add("dom0")
-            if ":" not in output_lines[1]:
+
+            second_line = output_lines[1] if len(output_lines) > 1 else ""
+            if ":" not in second_line:
                 return result
 
-            return result.union({
-                vm_name.strip()
-                for vm_name in output_lines[1]
-                .split(":", maxsplit=1)[1]
-                .split(",")
-            })
+            return result.union(
+                {
+                    vm_name.strip()
+                    for vm_name in second_line.split(":", maxsplit=1)[1].split(
+                        ","
+                    )
+                }
+            )
         except subprocess.CalledProcessError as err:
             if err.returncode != 100:
                 raise err

qui/updater/progress_page.py

@@ -24,7 +24,7 @@
 import threading
 import time
 import gi
-from typing import Dict, List
+from typing import Dict
 
 gi.require_version("Gtk", "3.0")  # isort:skip
 from gi.repository import Gtk, Gdk, GLib, GObject  # isort:skip
@@ -165,7 +165,7 @@ def update_selected(self, to_update, settings):
         self.update_details.update_buffer()
 
     def do_update_selected(
-            self, rows: Dict[str, RowWrapper], settings: Settings
+        self, rows: Dict[str, RowWrapper], settings: Settings
     ):
         """Runs `qubes-vm-update` command."""
         targets = ",".join((name for name in rows.keys()))
@@ -245,7 +245,9 @@ def handle_err_line(self, untrusted_line, rows):
 
     def read_stdouts(self, proc, rows):
         curr_name_out = ""
-        for untrusted_line in iter(proc.stdout.readline, "__COMMUNICATION_IS_DONE__"):
+        for untrusted_line in iter(
+            proc.stdout.readline, "__COMMUNICATION_IS_DONE__"
+        ):
             if untrusted_line != "__COMMUNICATION_IS_DONE__":
                 line = self._sanitize_line(untrusted_line)
                 maybe_name, text = line.split(" ", 1)

qui/updater/tests/conftest.py

@@ -18,7 +18,7 @@
 # You should have received a copy of the GNU Lesser General Public License along
 # with this program; if not, see <http://www.gnu.org/licenses/>.
 """Conftest helper pytest file: fixtures container here are
- reachable by all tests"""
+reachable by all tests"""
 import pytest
 import importlib.resources
 
@@ -87,7 +87,7 @@ def test_qapp_impl():
         qapp,
         "dom0",
         "AdminVM",
-        {},
+        {"updateable": ("bool", True, "True")},
         {
             "service.qubes-update-check": 1,
             "config.default.qubes-update-check": None,
@@ -337,7 +337,7 @@ def all_vms_list(test_qapp, mock_list_store):
 
 
 @pytest.fixture
-def updatable_vms_list(test_qapp, mock_list_store):
+def updateable_vms_list(test_qapp, mock_list_store):
     result = ListWrapper(UpdateRowWrapper, mock_list_store)
     for vm in test_qapp.domains:
         if vm.klass in ("AdminVM", "TemplateVM", "StandaloneVM"):

qui/updater/tests/test_intro_page.py

@@ -40,9 +40,7 @@ def test_populate_vm_list(
         ("test-standalone", "admin.vm.feature.Get", "updates-available", None)
     ] = (b"0\x00" + str(1).encode())
     # inconsistent output of qubes-vm-update, but it does not matter
-    mock_subprocess.return_value = (
-        b"Following templates will be updated:test-standalone"
-    )
+    mock_subprocess.return_value = b"The admin VM will not be updated.\nFollowing templates will be updated:test-standalone"
 
     assert not sut.is_populated
 
@@ -56,12 +54,13 @@ def test_populate_vm_list(
         ("fedora-36", "admin.vm.feature.Get", "updates-available", None)
     ] = (b"0\x00" + str(1).encode())
     mock_subprocess.return_value = (
+        b"The admin VM (dom0) will be updated.\n"
         b"Following templates will be updated:test-standalone,fedora-36"
     )
 
     sut.populate_vm_list(test_qapp, mock_settings)
     assert len(sut.list_store) == 4
-    assert len(sut.get_vms_to_update()) == 2
+    assert len(sut.get_vms_to_update()) == 3
 
 
 @pytest.mark.parametrize(
@@ -340,35 +339,52 @@ def test_prohibit_start_rationale_tooltip(
         # Comma separated list of VMs to target
         pytest.param(
             ("--targets", "dom0,fedora-36"),
-            b"fedora-36",
+            b"dom0,fedora-36",
             b"",
             {"dom0", "fedora-36"},
-            ("--targets", "fedora-36"),
+            ("--targets", "dom0,fedora-36"),
             id="targets",
         ),
         # `qubes-update-gui --standalones`
         # Target all StandaloneVMs
         pytest.param(
             ("--standalones",),
-            b"",
             ",".join(_standalones).encode(),
+            b"",
             _standalones,
             ("--standalones",),
             id="standalones",
         ),
         # `qubes-update-gui --dom0`
         # Target dom0
         pytest.param(
-            ("--dom0", "--force-update"), b"", b"", {"dom0"}, None, id="dom0"
+            ("--dom0", "--force-update"),
+            b"",
+            b"",
+            {"dom0"},
+            ("--force-update", "--targets", "dom0"),
+            id="dom0",
         ),
         # `qubes-update-gui --dom0 --skip dom0`
         # Comma separated list of VMs to be skipped,
         # works with all other options.
         pytest.param(
-            ("--dom0", "--skip", "dom0"), b"", b"", set(), None, id="skip all"
+            ("--dom0", "--skip", "dom0"),
+            b"",
+            b"",
+            set(),
+            ("--skip", "dom0", "--targets", "dom0"),
+            id="skip all",
         ),
         # `qubes-update-gui --skip dom0`
-        pytest.param(("--skip", "dom0"), b"", b"", set(), None, id="skip dom0"),
+        pytest.param(
+            ("--skip", "dom0"),
+            b"",
+            b"",
+            set(),
+            ("--skip", "dom0"),
+            id="skip dom0",
+        ),
         # `qubes-update-gui --skip garbage-name`
         pytest.param(
             ("--skip", "garbage-name"),
@@ -385,7 +401,7 @@ def test_prohibit_start_rationale_tooltip(
             b"",
             b"",
             set(),
-            None,
+            ("--skip", "dom0", "--targets", "dom0"),
             id="skip all targets",
         ),
         # `qubes-update-gui --templates dom0 --skip fedora-36,garbage-name`
@@ -426,7 +442,11 @@ def test_select_rows_ignoring_conditions(
 
     assert len(sut.list_store) == 12
 
-    result = b""
+    if "dom0" in expected_selection:
+        result = b"The admin VM (dom0) will be updated.\n"
+    else:
+        result = b"The admin VM will not be updated.\n"
+
     if tmpls_and_stndas:
         result += (
             b"Following templates and standalones will be updated: "
@@ -446,7 +466,7 @@ def test_select_rows_ignoring_conditions(
         ] = (b"0\x00" + b"3020-01-01 00:00:00")
 
     cliargs = parse_args(args, test_qapp)
-    sut.select_rows_ignoring_conditions(cliargs, test_qapp.domains["dom0"])
+    sut.select_rows_ignoring_conditions(cliargs)
     to_update = {row.name for row in sut.list_store if row.selected}
 
     assert to_update == expected_selection

qui/updater/tests/test_progress_page.py

@@ -84,7 +84,7 @@ def test_perform_update(
     mock_next_button,
     mock_cancel_button,
     mock_label,
-    updatable_vms_list,
+    updateable_vms_list,
 ):
     mock_log = Mock()
     mock_callback = Mock()
@@ -97,7 +97,7 @@ def test_perform_update(
         mock_callback,
     )
 
-    sut.vms_to_update = updatable_vms_list
+    sut.vms_to_update = updateable_vms_list
 
     class VMConsumer:
         def __call__(self, vm_rows, *args, **kwargs):
@@ -130,7 +130,7 @@ def test_update_templates(
     idle_add,
     interrupted,
     real_builder,
-    updatable_vms_list,
+    updateable_vms_list,
     mock_next_button,
     mock_cancel_button,
     mock_label,
@@ -154,15 +154,15 @@ def test_update_templates(
 
     sut.update_details.progress_textview = mock_text_view
     # chose vm to show details
-    sut.update_details.active_row = updatable_vms_list[0]
-    for i, row in enumerate(updatable_vms_list):
+    sut.update_details.active_row = updateable_vms_list[0]
+    for i, row in enumerate(updateable_vms_list):
         row.buffer = f"Details {i}"
 
     if interrupted:
         sut.interrupt_update()
-    sut.update_selected(updatable_vms_list, mock_settings)
+    sut.update_selected(updateable_vms_list, mock_settings)
 
-    sut.update_details.set_active_row(updatable_vms_list[2])
+    sut.update_details.set_active_row(updateable_vms_list[2])
 
     calls = [
         call(sut.set_total_progress, 100),
@@ -252,7 +252,7 @@ def test_get_update_summary(
     mock_next_button,
     mock_cancel_button,
     mock_label,
-    updatable_vms_list,
+    updateable_vms_list,
 ):
     mock_log = Mock()
     mock_callback = Mock()
@@ -265,12 +265,12 @@ def test_get_update_summary(
         mock_callback,
     )
 
-    updatable_vms_list[0].set_status(UpdateStatus.NoUpdatesFound)
-    updatable_vms_list[1].set_status(UpdateStatus.Error)
-    updatable_vms_list[2].set_status(UpdateStatus.Cancelled)
-    updatable_vms_list[3].set_status(UpdateStatus.Success)
+    updateable_vms_list[0].set_status(UpdateStatus.NoUpdatesFound)
+    updateable_vms_list[1].set_status(UpdateStatus.Error)
+    updateable_vms_list[2].set_status(UpdateStatus.Cancelled)
+    updateable_vms_list[3].set_status(UpdateStatus.Success)
 
-    sut.vms_to_update = updatable_vms_list
+    sut.vms_to_update = updateable_vms_list
 
     updated, no_updates, failed, cancelled = sut.get_update_summary()
 
@@ -281,9 +281,9 @@ def test_get_update_summary(
     mock_callback.assert_not_called()
 
 
-def test_set_active_row(real_builder, updatable_vms_list):
+def test_set_active_row(real_builder, updateable_vms_list):
     sut = QubeUpdateDetails(real_builder)
-    row = updatable_vms_list[0]
+    row = updateable_vms_list[0]
     sut.set_active_row(row)
 
     assert sut.details_label.get_text().strip() == "Details for"

qui/updater/tests/test_summary_page.py

@@ -201,7 +201,7 @@ def test_populate_restart_list(
     expected,
     real_builder,
     test_qapp,
-    updatable_vms_list,
+    updateable_vms_list,
     mock_next_button,
     mock_cancel_button,
     mock_settings,
@@ -224,7 +224,7 @@ def test_populate_restart_list(
     )
     sut.summary_list = mock_tree_view
 
-    for row in updatable_vms_list:
+    for row in updateable_vms_list:
         row.set_status(UpdateStatus.Success)
         if row.vm.klass == "TemplateVM":
             for i, appvm in enumerate(row.vm.appvms):
@@ -233,7 +233,7 @@ def test_populate_restart_list(
                 else:
                     appvm.is_running = lambda *_args: False
 
-    sut.populate_restart_list(True, updatable_vms_list, mock_settings)
+    sut.populate_restart_list(True, updateable_vms_list, mock_settings)
 
     assert len(sut.list_store) == UP_VMS
     assert sum(row.selected for row in sut.list_store) == expected

qui/updater/tests/test_updater.py

@@ -46,7 +46,7 @@ def test_setup_non_interactive_nothing_to_do(
     get_vms, select, subproc, _mock_logging, __mock_logging, test_qapp
 ):
     sut = QubesUpdater(test_qapp, parse_args(("-n",), test_qapp))
-    subproc.return_value = b""
+    subproc.return_value = b"The admin VM will not be updated."
     get_vms.return_value = ()
     sut.perform_setup()
     select.assert_called_once()

qui/updater/updater.py

@@ -204,7 +204,7 @@ def cell_data_func(_column, cell, model, it, data):
         if skip_intro_if_args(self.cliargs):
             self.log.info("Skipping intro page.")
             self.intro_page.select_rows_ignoring_conditions(
-                cliargs=self.cliargs, dom0=self.qapp.domains["dom0"]
+                cliargs=self.cliargs
             )
             if len(self.intro_page.get_vms_to_update()) == 0:
                 self.do_nothing = True
@@ -492,8 +492,7 @@ def parse_args(args, app):
     parser.add_argument(
         "--dom0",
         action="store_true",
-        help="Target dom0. "
-        "If present, skip manual selection of qubes to update.",
+        help="Target dom0. " "Short version of --targets dom0.",
     )
 
     parser.add_argument(