Repository: miurahr/aqtinstall

Issue:
Better error handling
I made a mistake typing the `aqt` command:

```
$ aqt install 5.12 mac desktop                                                                                                                                                  216ms
Finished installation
Traceback (most recent call last):
  File "/usr/local/bin/aqt", line 9, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.7/site-packages/aqt/__init__.py", line 28, in main
    return cli.run()
  File "/usr/local/lib/python3.7/site-packages/aqt/cli.py", line 218, in run
    args.func(args)
  File "/usr/local/lib/python3.7/site-packages/aqt/cli.py", line 117, in run_install
    QtInstaller(QtArchives(os_name, target, qt_version, arch, modules=modules, mirror=mirror, logging=self.logger),
  File "/usr/local/lib/python3.7/site-packages/aqt/archives.py", line 79, in __init__
    self._get_archives()
  File "/usr/local/lib/python3.7/site-packages/aqt/archives.py", line 100, in _get_archives
    self.update_xml = ElementTree.fromstring(r.text)
  File "/usr/local/Cellar/python/3.7.6_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/xml/etree/ElementTree.py", line 1315, in XML
    parser.feed(text)
xml.etree.ElementTree.ParseError: syntax error: line 1, column 49
```

I'm not a python developer and such error is hard to debug.

Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index cfbda92..2e90ce5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -7,6 +7,10 @@ matrix:
       env: TOXENV=check
     - python: 3.7
       env: TOXENV=docs
+    - python: 3.6
+      env:
+        TOXENV=py36
+        PYTEST_ADDOPTS="--remote-data"
 
 install: pip install tox-travis
 
diff --git a/aqt/archives.py b/aqt/archives.py
index 7d876fa..6de7b37 100644
--- a/aqt/archives.py
+++ b/aqt/archives.py
@@ -105,7 +105,17 @@ class QtArchives:
             self.logger.error('Download error: %s\n' % e.args, exc_info=True)
             raise e
         else:
+            if r.status_code != 200:
+                self.logger.error('Download error when access to {}\n'
+                                  'Server response code: {}, reason: {}'.format(update_xml_url,
+                                                                                r.status_code, r.reason))
+                exit(1)
+        try:
             self.update_xml = ElementTree.fromstring(r.text)
+        except ElementTree.ParseError as perror:
+            self.logger.error("Downloaded metadata is corrupted. {}".format(perror))
+            exit(1)
+        else:
             for packageupdate in self.update_xml.iter("PackageUpdate"):
                 name = packageupdate.find("Name").text
                 if self.all_extra or name in target_packages:
@@ -117,9 +127,9 @@ class QtArchives:
                             package_url = archive_url + name + "/" + full_version + archive
                             self.archives.append(QtPackage(name, package_url, archive, package_desc,
                                                            has_mirror=self.has_mirror))
-        if len(self.archives) == 0:
-            self.logger.error("Error while parsing package information!")
-            exit(1)
+            if len(self.archives) == 0:
+                self.logger.error("Error while parsing package information!")
+                exit(1)
 
     def get_archives(self):
         """
diff --git a/aqt/cli.py b/aqt/cli.py
index 0251ef5..fb8913b 100644
--- a/aqt/cli.py
+++ b/aqt/cli.py
@@ -55,6 +55,12 @@ class Cli():
                 return True
         return False
 
+    def _check_qt_arg_versions(self, qt_version):
+        for ver in self.settings.available_versions:
+            if ver == qt_version:
+                return True
+        return False
+
     def _set_sevenzip(self, args):
         sevenzip = args.external
         if sevenzip is None:
@@ -115,8 +121,11 @@ class Cli():
         if not self._check_mirror(mirror):
             self.parser.print_help()
             exit(1)
+        if not self._check_qt_arg_versions(qt_version):
+            self.logger.warning("Specified Qt version is unknown: {}.".format(qt_version))
         if not self._check_qt_arg_combination(qt_version, os_name, target, arch):
-            self.logger.warning("Specified target combination is not valid: {} {} {}".format(os_name, target, arch))
+            self.logger.warning("Specified target combination is not valid or unknown: {} {} {}".format(os_name,
+                                                                                                        target, arch))
         all_extra = True if modules is not None and 'all' in modules else False
         if not all_extra and not self._check_modules_arg(qt_version, modules):
             self.logger.warning("Some of specified modules are unknown.")
diff --git a/aqt/combinations.json b/aqt/combinations.json
index 7026609..4b7501a 100644
--- a/aqt/combinations.json
+++ b/aqt/combinations.json
@@ -89,4 +89,12 @@
     "qtquicktimeline", "qtscript", "qtvirtualkeyboard", "qtwebglplugin"]},
   {"qt_version": "5.14", "modules": ["qtcharts", "qtlottie", "qtnetworkauth", "qtpurchasing", "qtdatavis3d",
     "qtquick3d", "qtquicktimeline", "qtscript", "qtvirtualkeyboard", "qtwebglplugin"]}
+], "versions": [
+  "5.5", "5.6", "5.7", "5.8",
+  "5.9", "5.9.1", "5.9.2", "5.9.3", "5.9.4", "5.9.5", "5.9.6", "5.9.7", "5.9.8", "5.9.9",
+  "5.10.0", "5.10.1", "5.11.0", "5.11.1", "5.11.2", "5.11.3",
+  "5.12.0", "5.12.1", "5.12.2", "5.12.3", "5.12.4", "5.12.5", "5.12.6", "5.12.7",
+  "5.13.0", "5.13.1", "5.13.2",
+  "5.14.0", "5.14.1", "5.14.2",
+  "5.15.0"
 ]}]
diff --git a/aqt/installer.py b/aqt/installer.py
index 2e66e4d..bb31fcd 100644
--- a/aqt/installer.py
+++ b/aqt/installer.py
@@ -57,6 +57,7 @@ class QtInstaller:
     def retrieve_archive(self, package: QtPackage):
         archive = package.archive
         url = package.url
+        start_time = time.perf_counter()
         self.logger.info("Downloading {}...".format(url))
         try:
             r = requests.get(url, allow_redirects=False, stream=True)
@@ -84,7 +85,7 @@ class QtInstaller:
                 if self.command is not None:
                     self.extract_archive_ext(archive)
         os.unlink(archive)
-        self.logger.info("Finish installation of {} in {}".format(archive, time.process_time()))
+        self.logger.info("Finish installation of {} in {}".format(archive, time.perf_counter() - start_time))
 
     def extract_archive(self, archive):
         szf = py7zr.SevenZipFile(archive)
@@ -132,8 +133,10 @@ class QtInstaller:
     def install(self):
         with concurrent.futures.ThreadPoolExecutor() as executor:
             futures = [executor.submit(self.retrieve_archive, ar) for ar in self.qt_archives.get_archives()]
-            for future in concurrent.futures.as_completed(futures):
-                future.result()
+            done, not_done = concurrent.futures.wait(futures, return_when=concurrent.futures.FIRST_EXCEPTION)
+            if len(not_done) > 0:
+                self.logger.error("Installation error detected.")
+                exit(1)
 
         # finalize
         qt_version, target, arch = self.qt_archives.get_target_config()
diff --git a/aqt/settings.py b/aqt/settings.py
index 40c41a9..7e570f6 100644
--- a/aqt/settings.py
+++ b/aqt/settings.py
@@ -67,6 +67,10 @@ class Settings(object):
     def tools_combinations(self):
         return self._combinations['tools']
 
+    @property
+    def available_versions(self):
+        return self._combinations['versions']
+
     def available_modules(self, qt_version):
         """Known module names
 
diff --git a/requirements.txt b/requirements.txt
index a8687ee..1c1b1c2 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,7 @@
 requests
 pytest
 pytest-pep8
+pytest-remotedata
 flake8
 setuptools>=42
 setuptools-scm>=3.5.0
diff --git a/setup.cfg b/setup.cfg
index 4bf34a8..0ff6752 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -51,5 +51,6 @@ aqt = *.yml, *.json, *.ini
 dev =
     pytest
     pytest-pep8
+    pytest-remotedata
     pytest-cov
     flake8
diff --git a/tox.ini b/tox.ini
index 6f2aab6..4b6177d 100644
--- a/tox.ini
+++ b/tox.ini
@@ -2,7 +2,7 @@
 envlist = check, docs, py36
 
 [testenv]
-passenv = TRAVIS TRAVIS_*
+passenv = TRAVIS TRAVIS_* PYTEST_ADDOPTS
 basepython = python3
 deps = -rrequirements.txt
 commands =


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index dd154c9..cd55909 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -39,6 +39,12 @@ def test_cli_check_combination():
     assert not cli._check_qt_arg_combination('5.14.0', 'android', 'desktop', 'clang_64')
 
 
+def test_cli_check_version():
+    cli = aqt.cli.Cli()
+    assert cli._check_qt_arg_versions('5.12.0')
+    assert not cli._check_qt_arg_versions('5.12')
+
+
 def test_cli_check_mirror():
     cli = aqt.cli.Cli()
     assert cli._check_mirror(None)
diff --git a/tests/test_connection.py b/tests/test_connection.py
new file mode 100644
index 0000000..bc08bce
--- /dev/null
+++ b/tests/test_connection.py
@@ -0,0 +1,21 @@
+import pytest
+
+import aqt
+
+
+@pytest.mark.remote_data
+def test_cli_unknown_version(capsys):
+    wrong_version = "5.12"
+    wrong_url = "https://download.qt.io/online/qtsdkrepository/mac_x64/desktop/qt5_512/Updates.xml"
+    expected = ["aqt - WARNING - Specified Qt version is unknown: {}.".format(wrong_version),
+                "aqt - ERROR - Download error when access to {}"
+                " Server response code: 404, reason code: Not Found".format(wrong_url)
+                ]
+    with pytest.raises(SystemExit) as pytest_wrapped_e:
+        cli = aqt.cli.Cli()
+        cli.run(["install", wrong_version, "mac", "desktop"])
+    assert pytest_wrapped_e.type == SystemExit
+    assert pytest_wrapped_e.value.code == 1
+    out, err = capsys.readouterr()
+    for i, line in enumerate(out):
+        assert line.endswith(expected[i])