Fix misuse of disable_socket (#415)

In an earlier PR, I added calls to `disable_socket()` from
`pytest_socket` where I thought they were needed to prevent some tests
from accessing the network, in case they weren't monkeypatched properly.
Today, I discovered that `disable_socket()` disables sockets globally
for all tests, which means that the tests that use remote data cannot
run if they are executed after another test calls `disable_socket()`.

This change calls `disable_socket()` once from `conftest.py`, so that
no tests are allowed to use network data unless they are marked as ok
to use the network, with `@pytest.mark.enable_socket`. See example of
usage in `tests/test_connection.py`.

Changed return code for unexpected exceptions:

This allows us to write tests that can discover whether an unexpected
exception occurred just by checking the return code, rather than reading
stderr. This will allow us to write less friable tests that don't break
every time some insignificant output details change.

This change catches exceptions derived from Exception and
KeyboardInterrupt raised by `installer`, while run by multiple
processes, and propagates them back to earlier stack entries. This will
prevent any OSError and BrokenPipe exceptions that would otherwise be
raised when one process has an exception while the other processes are
still running.

This also handles the MemoryError exception we saw in #416, and offers
some suggestions for solving the issue.

aqt/installer.py

@@ -65,6 +65,8 @@ class Cli:
 
     __slot__ = ["parser", "combinations", "logger"]
 
+    UNHANDLED_EXCEPTION_CODE = 254
+
     def __init__(self):
         parser = argparse.ArgumentParser(
             prog="aqt",
@@ -113,7 +115,7 @@ def run(self, arg=None) -> int:
                 "Please file a bug report at https://github.com/miurahr/aqtinstall/issues.\n"
                 "Please remember to include a copy of this program's output in your report."
             )
-            return 1
+            return Cli.UNHANDLED_EXCEPTION_CODE
 
     def _check_tools_arg_combination(self, os_name, tool_name, arch):
         for c in Settings.tools_combinations:

tests/conftest.py

@@ -0,0 +1,5 @@
+from pytest_socket import disable_socket
+
+
+def pytest_runtest_setup():
+    disable_socket()

tests/test_cli.py

@@ -294,7 +294,7 @@ def _mocked_run(*args):
 
     cli = Cli()
     cli._setup_settings()
-    assert 1 == cli.run(["install-qt", "mac", "ios", "6.2.0"])
+    assert Cli.UNHANDLED_EXCEPTION_CODE == cli.run(["install-qt", "mac", "ios", "6.2.0"])
     out, err = capsys.readouterr()
     assert err.startswith("Some unexpected error")
     assert err.rstrip().endswith(

tests/test_connection.py

@@ -6,6 +6,7 @@
 import aqt
 
 
+@pytest.mark.enable_socket
 @pytest.mark.remote_data
 def test_cli_unknown_version(capsys):
     wrong_version = "5.16.0"

tests/test_helper.py

@@ -6,7 +6,6 @@
 
 import pytest
 import requests
-from pytest_socket import disable_socket
 from requests.models import Response
 
 from aqt import helper
@@ -15,12 +14,6 @@
 from aqt.metadata import Version
 
 
-@pytest.fixture(autouse=True)
-def disable_sockets():
-    # This blocks all network connections, causing test failure if we used monkeypatch wrong
-    disable_socket()
-
-
 def test_helper_altlink(monkeypatch):
     class Message:
         headers = {"content-type": "text/plain", "length": 300}

tests/test_install.py

@@ -12,7 +12,6 @@
 
 import py7zr
 import pytest
-from pytest_socket import disable_socket
 
 from aqt.archives import QtPackage
 from aqt.exceptions import ArchiveDownloadError, ArchiveExtractionError
@@ -155,10 +154,7 @@ def locate_archive() -> MockArchive:
 
 
 @pytest.fixture(autouse=True)
-def disable_sockets_and_multiprocessing(monkeypatch):
-    # This blocks all network connections, causing test failure if we used monkeypatch wrong
-    disable_socket()
-
+def disable_multiprocessing(monkeypatch):
     # This blocks all multiprocessing, which would otherwise spawn processes that are not monkeypatched
     monkeypatch.setattr(
         "aqt.installer.multiprocessing.get_context",
@@ -538,41 +534,45 @@ def mock_get_url(url, *args, **kwargs):
 
 
 @pytest.mark.parametrize(
-    "make_exception, expect_end_msg, settings_file",
+    "make_exception, settings_file, expect_end_msg, expect_return",
     (
         (
             "RuntimeError()",
+            "../aqt/settings.ini",
             "===========================PLEASE FILE A BUG REPORT===========================\n"
             "You have discovered a bug in aqt.\n"
             "Please file a bug report at https://github.com/miurahr/aqtinstall/issues.\n"
             "Please remember to include a copy of this program's output in your report.",
-            "../aqt/settings.ini",
+            Cli.UNHANDLED_EXCEPTION_CODE,
         ),
         (
             "KeyboardInterrupt()",
-            "Caught KeyboardInterrupt, terminating installer workers\n" "Installer halted by keyboard interrupt.",
             "../aqt/settings.ini",
+            "Caught KeyboardInterrupt, terminating installer workers\nInstaller halted by keyboard interrupt.",
+            1,
         ),
         (
             "MemoryError()",
+            "../aqt/settings.ini",
             "Caught MemoryError, terminating installer workers\n"
             "Out of memory when downloading and extracting archives in parallel.\n"
             "==============================Suggested follow-up:==============================\n"
             "* Please reduce your 'concurrency' setting (see "
             "https://aqtinstall.readthedocs.io/en/stable/configuration.html#configuration)",
-            "../aqt/settings.ini",
+            1,
         ),
         (
             "MemoryError()",
+            "data/settings_no_concurrency.ini",
             "Caught MemoryError, terminating installer workers\n"
             "Out of memory when downloading and extracting archives.\n"
             "==============================Suggested follow-up:==============================\n"
             "* Please free up more memory.",
-            "data/settings_no_concurrency.ini",
+            1,
         ),
     ),
 )
-def test_install_pool_exception(monkeypatch, capsys, make_exception, expect_end_msg, settings_file):
+def test_install_pool_exception(monkeypatch, capsys, make_exception, settings_file, expect_end_msg, expect_return):
     def mock_installer_func(*args):
         raise eval(make_exception)
 
@@ -588,7 +588,7 @@ def mock_installer_func(*args):
 
     Settings.load_settings(str(Path(__file__).parent / settings_file))
     cli = Cli()
-    assert cli.run(cmd) == 1
+    assert expect_return == cli.run(cmd)
     out, err = capsys.readouterr()
     assert err.rstrip().endswith(expect_end_msg)
 

tests/test_updater.py

@@ -2,20 +2,13 @@
 from tempfile import TemporaryDirectory
 
 import pytest
-from pytest_socket import disable_socket
 
 from aqt.archives import TargetConfig
 from aqt.exceptions import UpdaterError
 from aqt.helper import Settings
 from aqt.updater import Updater
 
 
-@pytest.fixture(autouse=True)
-def disable_sockets():
-    # This blocks all network connections, causing test failure if we used monkeypatch wrong
-    disable_socket()
-
-
 @pytest.fixture(autouse=True)
 def setup_settings():
     Settings.load_settings()