Repository: miurahr/aqtinstall
Pull request #402 (2021-09-20 20:37:27)

Add helpful error messages when modules/tools/Qt version does not exist
This adds error messages that suggest using `aqt list-*` when a user attempts to install:
1. Versions of Qt that don't exist
2. Modules that don't exist
3. Tools that don't exist
4. Tool variants that don't exist

See changes to `tests/test_install.py` for examples of the expected error messages.

The idea here is that the user will be encouraged to use `aqt list-qt` and `aqt list-tool` when one of these errors occur, instead of trying to figure out why a 404 occurred, and trying to debug `aqt` to figure out what went wrong.

There's one thing that I didn't do that could potentially improve this: We could automatically run the suggested `aqt list-*` command and print "These are the available values for the parameter you got wrong: [...]" in the error message, instead of asking the user to run `aqt list-*`.


Patch:
diff --git a/aqt/archives.py b/aqt/archives.py
index 4bc857b..3c06d8c 100644
--- a/aqt/archives.py
+++ b/aqt/archives.py
@@ -26,7 +26,7 @@ from dataclasses import dataclass, field
 from logging import getLogger
 from typing import Dict, Iterable, List, Optional, Tuple
 
-from aqt.exceptions import ArchiveListError, NoPackageFound
+from aqt.exceptions import ArchiveDownloadError, ArchiveListError, NoPackageFound
 from aqt.helper import Settings, getUrl
 from aqt.metadata import QtRepoProperty, Version
 
@@ -105,11 +105,14 @@ class ModuleToPackage:
     def has_package(self, package_name: str):
         return package_name in self._packages_to_modules
 
+    def get_modules(self) -> Iterable[str]:
+        return self._modules_to_packages.keys()
+
     def __len__(self) -> int:
         return len(self._modules_to_packages)
 
     def __format__(self, format_spec) -> str:
-        return str(set(self._modules_to_packages.keys()))
+        return str(sorted(set(self._modules_to_packages.keys())))
 
 
 class QtArchives:
@@ -142,10 +145,18 @@ class QtArchives:
         self.archives: List[QtPackage] = []
         self.mod_list: Iterable[str] = modules or []
         self.timeout = timeout
-        self._get_archives()
+        try:
+            self._get_archives()
+        except ArchiveDownloadError as e:
+            self.handle_missing_updates_xml(e)
         if not all_archives:
             self.archives = list(filter(lambda a: a.name in subarchives, self.archives))
 
+    def handle_missing_updates_xml(self, e: ArchiveDownloadError):
+        msg = f"Failed to locate XML data for Qt version '{self.version}'."
+        help_msg = f"Please use 'aqt list-qt {self.os_name} {self.target}' to show versions available."
+        raise ArchiveListError(msg, suggested_action=[help_msg]) from e
+
     def _version_str(self) -> str:
         return ("{0.major}{0.minor}" if self.version == Version("5.9.0") else "{0.major}{0.minor}{0.patch}").format(
             self.version
@@ -257,7 +268,20 @@ class QtArchives:
         # if we have located every requested package, then target_packages will be empty
         if len(target_packages) > 0:
             message = f"The packages {target_packages} were not found while parsing XML of package information!"
-            raise NoPackageFound(message)
+            raise NoPackageFound(message, suggested_action=self.help_msg(target_packages.get_modules()))
+
+    def help_msg(self, missing_modules: Iterable[str]) -> Iterable[str]:
+        base_cmd = f"aqt list-qt {self.os_name} {self.target}"
+        arch = f"Please use '{base_cmd} --arch {self.version}' to show architectures available."
+        mods = f"Please use '{base_cmd} --modules {self.version} <arch>' to show modules available."
+        has_base_pkg: bool = self._base_target_package_name() in missing_modules
+        has_non_base_pkg: bool = len(list(missing_modules)) > 1 or not has_base_pkg
+        messages = []
+        if has_base_pkg:
+            messages.append(arch)
+        if has_non_base_pkg:
+            messages.append(mods)
+        return messages
 
     def get_packages(self) -> List[QtPackage]:
         """
@@ -359,6 +383,11 @@ class ToolArchives(QtArchives):
     def __str__(self):
         return f"ToolArchives(tool_name={self.tool_name}, version={self.version}, arch={self.arch})"
 
+    def handle_missing_updates_xml(self, e: ArchiveDownloadError):
+        msg = f"Failed to locate XML data for the tool '{self.tool_name}'."
+        help_msg = f"Please use 'aqt list-tool {self.os_name} {self.target}' to show tools available."
+        raise ArchiveListError(msg, suggested_action=[help_msg]) from e
+
     def _get_archives(self):
         _a = "_x64"
         if self.os_name == "windows":
@@ -387,15 +416,15 @@ class ToolArchives(QtArchives):
         try:
             packageupdate = next(filter(lambda x: x.find("Name").text == self.arch, self.update_xml.iter("PackageUpdate")))
         except StopIteration:
-            message = f"The package {self.arch} was not found while parsing XML of package information!"
-            raise NoPackageFound(message)
+            message = f"The package '{self.arch}' was not found while parsing XML of package information!"
+            raise NoPackageFound(message, suggested_action=self.help_msg())
 
         name = packageupdate.find("Name").text
         named_version = packageupdate.find("Version").text
         package_desc = packageupdate.find("Description").text
         downloadable_archives = packageupdate.find("DownloadableArchives").text
         if not downloadable_archives:
-            message = f"The package {self.arch} contains no downloadable archives!"
+            message = f"The package '{self.arch}' contains no downloadable archives!"
             raise NoPackageFound(message)
         for archive in downloadable_archives.split(", "):
             package_url = posixpath.join(
@@ -418,6 +447,9 @@ class ToolArchives(QtArchives):
                 )
             )
 
+    def help_msg(self, *args) -> Iterable[str]:
+        return [f"Please use 'aqt list-tool {self.os_name} {self.target} {self.tool_name}' to show tool variants available."]
+
     def get_target_config(self) -> TargetConfig:
         """Get target configuration.
 
diff --git a/aqt/installer.py b/aqt/installer.py
index ed68f95..b0ecad4 100644
--- a/aqt/installer.py
+++ b/aqt/installer.py
@@ -37,9 +37,17 @@ from typing import Any, Callable, List, Optional
 
 import aqt
 from aqt.archives import QtArchives, QtPackage, SrcDocExamplesArchives, ToolArchives
-from aqt.exceptions import AqtException, ArchiveConnectionError, ArchiveExtractionError, CliInputError, CliKeyboardInterrupt
+from aqt.exceptions import (
+    AqtException,
+    ArchiveConnectionError,
+    ArchiveDownloadError,
+    ArchiveExtractionError,
+    ArchiveListError,
+    CliInputError,
+    CliKeyboardInterrupt,
+)
 from aqt.helper import MyQueueListener, Settings, downloadBinaryFile, getUrl, setup_logging
-from aqt.metadata import ArchiveId, MetadataFactory, QtRepoProperty, SimpleSpec, Version, show_list
+from aqt.metadata import ArchiveId, MetadataFactory, QtRepoProperty, SimpleSpec, Version, show_list, suggested_follow_up
 from aqt.updater import Updater
 
 try:
@@ -401,11 +409,14 @@ class Cli:
         else:
             timeout = (Settings.connection_timeout, Settings.response_timeout)
         if args.arch is None:
-            archs = MetadataFactory(
-                archive_id=ArchiveId("tools", os_name, target, ""),
-                is_latest_version=True,
-                tool_name=tool_name,
-            ).getList()
+            archive_id = ArchiveId("tools", os_name, target, "")
+            meta = MetadataFactory(archive_id, is_latest_version=True, tool_name=tool_name)
+            try:
+                archs = meta.getList()
+            except ArchiveDownloadError as e:
+                msg = f"Failed to locate XML data for the tool '{tool_name}'."
+                raise ArchiveListError(msg, suggested_action=suggested_follow_up(meta)) from e
+
         else:
             archs = [args.arch]
 


Test patch:
diff --git a/tests/test_archives.py b/tests/test_archives.py
index f4fe90c..66020e0 100644
--- a/tests/test_archives.py
+++ b/tests/test_archives.py
@@ -145,12 +145,14 @@ def test_qt_archives_modules(monkeypatch, arch, requested_module_names, has_none
         assert len(expected_7z_files) == 0, "Actual number of packages was fewer than expected"
 
     if has_nonexistent_modules:
+        expect_help = [f"Please use 'aqt list-qt {os_name} {target} --modules {version} <arch>' to show modules available."]
         for unexpected_module in requested_module_names:
             with pytest.raises(NoPackageFound) as e:
                 mod_names = ("qtcharts", unexpected_module)
                 QtArchives(os_name, target, str(version), arch, base, modules=mod_names)
             assert e.type == NoPackageFound
             assert unexpected_module in str(e.value), "Message should include the missing module"
+            assert e.value.suggested_action == expect_help
         return
 
     is_all_modules = "all" in requested_module_names
@@ -197,10 +199,12 @@ def test_tools_variants(monkeypatch, tool_name, tool_variant_name, is_expect_fai
     monkeypatch.setattr(QtArchives, "_download_update_xml", _mock)
 
     if is_expect_fail:
+        expect_help = [f"Please use 'aqt list-tool {host} {target} {tool_name}' to show tool variants available."]
         with pytest.raises(NoPackageFound) as e:
             ToolArchives(host, target, tool_name, base, arch=tool_variant_name)
         assert e.type == NoPackageFound
         assert tool_variant_name in str(e.value), "Message should include the missing variant"
+        assert e.value.suggested_action == expect_help
         return
 
     expect_json = json.loads((Path(__file__).parent / "data" / f"{datafile}-expect.json").read_text("utf-8"))
diff --git a/tests/test_connection.py b/tests/test_connection.py
index d6f6fbf..8d29d13 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -9,7 +9,6 @@ import aqt
 @pytest.mark.remote_data
 def test_cli_unknown_version(capsys):
     wrong_version = "5.16.0"
-    wrong_url_ending = "mac_x64/desktop/qt5_5160/Updates.xml"
     cli = aqt.installer.Cli()
     assert cli.run(["install-qt", "mac", "desktop", wrong_version]) == 1
     out, err = capsys.readouterr()
@@ -22,24 +21,26 @@ def test_cli_unknown_version(capsys):
 
     aqtinstall(aqt) v.* on Python 3.*
     Specified Qt version is unknown: 5.16.0.
-    Failed to retrieve file at https://download.qt.io/online/qtsdkrepository/mac_x64/desktop/qt5_5160/Updates.xml
-    Server response code: 404, reason: Not Found
+    Failed to locate XML data for Qt version '5.16.0'.
+    ==============================Suggested follow-up:==============================
+    * Please use 'aqt list-qt mac desktop' to show versions available.
 
     Expected result when redirect occurs:
 
     aqtinstall(aqt) v.* on Python 3.*
     Specified Qt version is unknown: 5.16.0.
     Connection to the download site failed and fallback to mirror site.
-    Failed to retrieve file at .*/mac_x64/desktop/qt5_5160/Updates.xml
-    Server response code: 404, reason: Not Found
-    Connection to the download site failed. Aborted...
+    Failed to locate XML data for Qt version '5.16.0'.
+    ==============================Suggested follow-up:==============================
+    * Please use 'aqt list-qt mac desktop' to show versions available.
     """
 
     matcher = re.compile(
         r"^aqtinstall\(aqt\) v.* on Python 3.*\n"
         r".*Specified Qt version is unknown: " + re.escape(wrong_version) + r"\.\n"
-        r".*Failed to retrieve file at .*" + re.escape(wrong_url_ending) + r"\n"
-        r".*Server response code: 404, reason: Not Found.*"
+        r".*Failed to locate XML data for Qt version '" + re.escape(wrong_version) + r"'\.\n"
+        r"==============================Suggested follow-up:==============================\n"
+        r"\* Please use 'aqt list-qt mac desktop' to show versions available\.\n",
     )
 
     assert matcher.match(err)
diff --git a/tests/test_install.py b/tests/test_install.py
index f38455c..b9837b9 100644
--- a/tests/test_install.py
+++ b/tests/test_install.py
@@ -15,7 +15,7 @@ import pytest
 from pytest_socket import disable_socket
 
 from aqt.archives import QtPackage
-from aqt.exceptions import ArchiveExtractionError
+from aqt.exceptions import ArchiveDownloadError, ArchiveExtractionError
 from aqt.installer import Cli, installer
 
 
@@ -434,7 +434,7 @@ def test_install(
         cli = Cli()
         cli._setup_settings()
 
-        cli.run(cmd + ["--outputdir", output_dir])
+        assert 0 == cli.run(cmd + ["--outputdir", output_dir])
 
         out, err = capsys.readouterr()
         sys.stdout.write(out)
@@ -457,58 +457,83 @@ def test_install(
 
 
 @pytest.mark.parametrize(
-    "cmd, xml_file, expected_re, expect_missing",
+    "cmd, xml_file, expected",
     (
+        (
+            "install-qt windows desktop 5.16.0 win32_mingw73",
+            None,
+            "Specified Qt version is unknown: 5.16.0.\n"
+            "Failed to locate XML data for Qt version '5.16.0'.\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-qt windows desktop' to show versions available.\n",
+        ),
+        (
+            "install-qt windows desktop 5.15.0 bad_arch",
+            "windows-5150-update.xml",
+            "Specified target combination is not valid or unknown: windows desktop bad_arch\n"
+            "The packages ['qt_base'] were not found while parsing XML of package information!\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-qt windows desktop --arch 5.15.0' to show architectures available.\n",
+        ),
         (
             "install-qt windows desktop 5.15.0 win32_mingw73 -m nonexistent foo",
             "windows-5150-update.xml",
-            # Match output: Note that the set of missing package names will be in random order.
-            re.compile(
-                r"^aqtinstall\(aqt\) v.+\n"
-                r"Some of specified modules are unknown\.\n"
-                r"The packages {(.+)} were not found while parsing XML of package information!$"
-            ),
-            {"qt_base", "nonexistent", "foo"},
+            "Some of specified modules are unknown.\n"
+            "The packages ['foo', 'nonexistent', 'qt_base'] were not found while parsing XML of package information!\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-qt windows desktop --arch 5.15.0' to show architectures available.\n"
+            "* Please use 'aqt list-qt windows desktop --modules 5.15.0 <arch>' to show modules available.\n",
         ),
         (
             "install-src windows desktop 5.15.0 -m nonexistent foo",
             "windows-5152-src-doc-example-update.xml",
-            # Match output: Note that the set of missing package names will be in random order.
-            re.compile(
-                r"^aqtinstall\(aqt\) v.+\n" r"The packages {(.+)} were not found while parsing XML of package information!$"
-            ),
-            {"src", "nonexistent", "foo"},
+            "The packages ['foo', 'nonexistent', 'src'] were not found while parsing XML of package information!\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-qt windows desktop --arch 5.15.0' to show architectures available.\n"
+            "* Please use 'aqt list-qt windows desktop --modules 5.15.0 <arch>' to show modules available.\n",
+        ),
+        (
+            "install-tool windows desktop tools_vcredist nonexistent",
+            "windows-desktop-tools_vcredist-update.xml",
+            "Specified target combination is not valid: windows tools_vcredist nonexistent\n"
+            "The package 'nonexistent' was not found while parsing XML of package information!\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-tool windows desktop tools_vcredist' to show tool variants available.\n",
+        ),
+        (
+            "install-tool windows desktop tools_nonexistent nonexistent",
+            None,
+            "Specified target combination is not valid: windows tools_nonexistent nonexistent\n"
+            "Failed to locate XML data for the tool 'tools_nonexistent'.\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-tool windows desktop' to show tools available.\n",
+        ),
+        (
+            "install-tool windows desktop tools_nonexistent",
+            None,
+            "Failed to locate XML data for the tool 'tools_nonexistent'.\n"
+            "==============================Suggested follow-up:==============================\n"
+            "* Please use 'aqt list-tool windows desktop' to check what tools are available.\n",
         ),
-        # (
-        #     "install-tool windows desktop tools_nonexistent nonexistent",
-        #     # Match output: Note that the set of missing package names will be in random order.
-        #     re.compile(
-        #         r"^aqtinstall\(aqt\) v.+\n"
-        #         r"Specified target combination is not valid: windows tools_nonexistent nonexistent\n"
-        #         r"The packages {(.+)} were not found while parsing XML of package information!$"
-        #     ),
-        #     {'nonexistent'}
-        # )
     ),
 )
-def test_install_bad_tools_modules(monkeypatch, capsys, cmd, xml_file, expected_re, expect_missing):
-    xml = (Path(__file__).parent / "data" / xml_file).read_text("utf-8")
-
-    def mock_get_url(self, url):
-        return xml
+def test_install_nonexistent_archives(monkeypatch, capsys, cmd, xml_file: Optional[str], expected):
+    def mock_get_url(url, *args, **kwargs):
+        if not xml_file:
+            raise ArchiveDownloadError(f"Failed to retrieve file at {url}\nServer response code: 404, reason: Not Found")
+        return (Path(__file__).parent / "data" / xml_file).read_text("utf-8")
 
     monkeypatch.setattr("aqt.archives.getUrl", mock_get_url)
     monkeypatch.setattr("aqt.installer.getUrl", mock_get_url)
+    monkeypatch.setattr("aqt.metadata.getUrl", mock_get_url)
 
     cli = Cli()
     cli._setup_settings()
     assert cli.run(cmd.split()) == 1
 
     out, err = capsys.readouterr()
-    match = expected_re.match(err)
-    assert match
-    actual = set(map(lambda x: x[1:-1], match.group(1).split(", ")))
-    assert actual == expect_missing
+    actual = err[err.index("\n") + 1 :]
+    assert actual == expected, "{0} != {1}".format(actual, expected)
 
 
 def test_install_keyboard_interrupt(monkeypatch, capsys):