From f86cbed769fda5fa2de8eef092270993f32db5eb Mon Sep 17 00:00:00 2001
From: John Litborn <11260241+jakkdl@users.noreply.github.com>
Date: Sun, 17 Sep 2023 10:00:20 +0200
Subject: [PATCH] make imports in tests on libraries not in install_requires
 skip on ImportError (#2784)

* make imports in tests on libraries not in install_requires skip on ImportError

* change check for gen_exports, add black import

* add --skip-optional-imports

* add a run with no test-requirements to CI
---
 .github/workflows/ci.yml              |  6 ++++++
 ci.sh                                 | 12 ++++++++++--
 trio/_tests/pytest_plugin.py          | 20 +++++++++++++++++++-
 trio/_tests/test_dtls.py              | 11 +++++++++--
 trio/_tests/test_exports.py           | 25 +++++++++++++++++++++----
 trio/_tests/test_ssl.py               | 10 ++++++++--
 trio/_tests/tools/test_gen_exports.py | 21 +++++++++++++++++++++
 trio/_tools/gen_exports.py            | 11 +++++++++--
 8 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index d5aeb3ec04..998590ab73 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -96,11 +96,16 @@ jobs:
       matrix:
         python: ['pypy-3.9', 'pypy-3.10', '3.8', '3.9', '3.10', '3.11', '3.12-dev', 'pypy-3.9-nightly', 'pypy-3.10-nightly']
         check_formatting: ['0']
+        no_test_requirements: ['0']
         extra_name: ['']
         include:
           - python: '3.8'
             check_formatting: '1'
             extra_name: ', check formatting'
+          # separate test run that doesn't install test-requirements.txt
+          - python: '3.8'
+            no_test_requirements: '1'
+            extra_name: ', no test-requirements'
     continue-on-error: >-
       ${{
         (
@@ -129,6 +134,7 @@ jobs:
         run: ./ci.sh
         env:
           CHECK_FORMATTING: '${{ matrix.check_formatting }}'
+          NO_TEST_REQUIREMENTS: '${{ matrix.no_test_requirements }}'
       - if: always()
         uses: codecov/codecov-action@v3
         with:
diff --git a/ci.sh b/ci.sh
index e469c0243a..863564a943 100755
--- a/ci.sh
+++ b/ci.sh
@@ -49,7 +49,15 @@ if [ "$CHECK_FORMATTING" = "1" ]; then
     source check.sh
 else
     # Actual tests
-    python -m pip install -r test-requirements.txt
+    # expands to 0 != 1 if NO_TEST_REQUIREMENTS is not set, if set the `-0` has no effect
+    # https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
+    if [ ${NO_TEST_REQUIREMENTS-0} == 1 ]; then
+        python -m pip install pytest coverage
+        flags="--skip-optional-imports"
+    else
+        python -m pip install -r test-requirements.txt
+        flags=""
+    fi
 
     # So we can run the test for our apport/excepthook interaction working
     if [ -e /etc/lsb-release ] && grep -q Ubuntu /etc/lsb-release; then
@@ -123,7 +131,7 @@ else
 
     echo "::endgroup::"
     echo "::group:: Run Tests"
-    if COVERAGE_PROCESS_START=$(pwd)/../.coveragerc coverage run --rcfile=../.coveragerc -m pytest -r a -p trio._tests.pytest_plugin --junitxml=../test-results.xml --run-slow ${INSTALLDIR} --verbose --durations=10; then
+    if COVERAGE_PROCESS_START=$(pwd)/../.coveragerc coverage run --rcfile=../.coveragerc -m pytest -r a -p trio._tests.pytest_plugin --junitxml=../test-results.xml --run-slow ${INSTALLDIR} --verbose --durations=10 $flags; then
         PASSED=true
     else
         PASSED=false
diff --git a/trio/_tests/pytest_plugin.py b/trio/_tests/pytest_plugin.py
index 2170a1e8b6..bbb5093be5 100644
--- a/trio/_tests/pytest_plugin.py
+++ b/trio/_tests/pytest_plugin.py
@@ -1,19 +1,30 @@
+from __future__ import annotations
+
 import inspect
+from typing import NoReturn
 
 import pytest
 
 from ..testing import MockClock, trio_test
 
 RUN_SLOW = True
+SKIP_OPTIONAL_IMPORTS = False
 
 
 def pytest_addoption(parser: pytest.Parser) -> None:
     parser.addoption("--run-slow", action="store_true", help="run slow tests")
+    parser.addoption(
+        "--skip-optional-imports",
+        action="store_true",
+        help="skip tests that rely on libraries not required by trio itself",
+    )
 
 
 def pytest_configure(config: pytest.Config) -> None:
     global RUN_SLOW
-    RUN_SLOW = config.getoption("--run-slow", True)
+    RUN_SLOW = config.getoption("--run-slow", default=True)
+    global SKIP_OPTIONAL_IMPORTS
+    SKIP_OPTIONAL_IMPORTS = config.getoption("--skip-optional-imports", default=False)
 
 
 @pytest.fixture
@@ -34,3 +45,10 @@ def autojump_clock() -> MockClock:
 def pytest_pyfunc_call(pyfuncitem: pytest.Function) -> None:
     if inspect.iscoroutinefunction(pyfuncitem.obj):
         pyfuncitem.obj = trio_test(pyfuncitem.obj)
+
+
+def skip_if_optional_else_raise(error: ImportError) -> NoReturn:
+    if SKIP_OPTIONAL_IMPORTS:
+        pytest.skip(error.msg, allow_module_level=True)
+    else:  # pragma: no cover
+        raise error
diff --git a/trio/_tests/test_dtls.py b/trio/_tests/test_dtls.py
index b7cb1830d1..a077451c4c 100644
--- a/trio/_tests/test_dtls.py
+++ b/trio/_tests/test_dtls.py
@@ -8,8 +8,15 @@
 
 import attr
 import pytest
-import trustme
-from OpenSSL import SSL
+
+from trio._tests.pytest_plugin import skip_if_optional_else_raise
+
+try:
+    import trustme
+    from OpenSSL import SSL
+except ImportError as error:
+    skip_if_optional_else_raise(error)
+
 
 import trio
 import trio.testing
diff --git a/trio/_tests/test_exports.py b/trio/_tests/test_exports.py
index 2f1157db06..70ac242171 100644
--- a/trio/_tests/test_exports.py
+++ b/trio/_tests/test_exports.py
@@ -17,6 +17,7 @@
 
 import trio
 import trio.testing
+from trio._tests.pytest_plugin import skip_if_optional_else_raise
 
 from .. import _core, _util
 from .._core._tests.tutil import slow
@@ -33,7 +34,10 @@
 
 def _ensure_mypy_cache_updated():
     # This pollutes the `empty` dir. Should this be changed?
-    from mypy.api import run
+    try:
+        from mypy.api import run
+    except ImportError as error:
+        skip_if_optional_else_raise(error)
 
     global mypy_cache_updated
     if not mypy_cache_updated:
@@ -130,13 +134,19 @@ def no_underscores(symbols):
             py_typed_path.write_text("")
 
     if tool == "pylint":
-        from pylint.lint import PyLinter
+        try:
+            from pylint.lint import PyLinter
+        except ImportError as error:
+            skip_if_optional_else_raise(error)
 
         linter = PyLinter()
         ast = linter.get_ast(module.__file__, modname)
         static_names = no_underscores(ast)
     elif tool == "jedi":
-        import jedi
+        try:
+            import jedi
+        except ImportError as error:
+            skip_if_optional_else_raise(error)
 
         # Simulate typing "import trio; trio.<TAB>"
         script = jedi.Script(f"import {modname}; {modname}.")
@@ -172,6 +182,10 @@ def no_underscores(symbols):
     elif tool == "pyright_verifytypes":
         if not RUN_SLOW:  # pragma: no cover
             pytest.skip("use --run-slow to check against mypy")
+        try:
+            import pyright  # noqa: F401
+        except ImportError as error:
+            skip_if_optional_else_raise(error)
         import subprocess
 
         res = subprocess.run(
@@ -354,7 +368,10 @@ def lookup_symbol(symbol):
         )
 
         if tool == "jedi":
-            import jedi
+            try:
+                import jedi
+            except ImportError as error:
+                skip_if_optional_else_raise(error)
 
             script = jedi.Script(
                 f"from {module_name} import {class_name}; {class_name}."
diff --git a/trio/_tests/test_ssl.py b/trio/_tests/test_ssl.py
index f91cea8549..d92e35b0d9 100644
--- a/trio/_tests/test_ssl.py
+++ b/trio/_tests/test_ssl.py
@@ -9,8 +9,14 @@
 from functools import partial
 
 import pytest
-import trustme
-from OpenSSL import SSL
+
+from trio._tests.pytest_plugin import skip_if_optional_else_raise
+
+try:
+    import trustme
+    from OpenSSL import SSL
+except ImportError as error:
+    skip_if_optional_else_raise(error)
 
 import trio
 
diff --git a/trio/_tests/tools/test_gen_exports.py b/trio/_tests/tools/test_gen_exports.py
index e7d8ab94f2..007bfbb097 100644
--- a/trio/_tests/tools/test_gen_exports.py
+++ b/trio/_tests/tools/test_gen_exports.py
@@ -2,6 +2,15 @@
 import sys
 
 import pytest
+from trio._tests.pytest_plugin import skip_if_optional_else_raise
+
+# imports in gen_exports that are not in `install_requires` in setup.py
+try:
+    import astor  # noqa: F401
+    import isort  # noqa: F401
+except ImportError as error:
+    skip_if_optional_else_raise(error)
+
 
 from trio._tools.gen_exports import (
     File,
@@ -80,6 +89,12 @@ def test_create_pass_through_args():
 @skip_lints
 @pytest.mark.parametrize("imports", ["", IMPORT_1, IMPORT_2, IMPORT_3])
 def test_process(tmp_path, imports):
+    try:
+        import black  # noqa: F401
+    # there's no dedicated CI run that has astor+isort, but lacks black.
+    except ImportError as error:  # pragma: no cover
+        skip_if_optional_else_raise(error)
+
     modpath = tmp_path / "_module.py"
     genpath = tmp_path / "_generated_module.py"
     modpath.write_text(SOURCE, encoding="utf-8")
@@ -107,6 +122,12 @@ def test_process(tmp_path, imports):
 @skip_lints
 def test_lint_failure(tmp_path) -> None:
     """Test that processing properly fails if black or isort does."""
+    try:
+        import black  # noqa: F401
+    # there's no dedicated CI run that has astor+isort, but lacks black.
+    except ImportError as error:  # pragma: no cover
+        skip_if_optional_else_raise(error)
+
     file = File(tmp_path / "module.py", "module")
 
     with pytest.raises(SystemExit):
diff --git a/trio/_tools/gen_exports.py b/trio/_tools/gen_exports.py
index 6549d473ab..47383c7055 100755
--- a/trio/_tools/gen_exports.py
+++ b/trio/_tools/gen_exports.py
@@ -16,11 +16,14 @@
 from textwrap import indent
 from typing import TYPE_CHECKING
 
+import attr
+
 if TYPE_CHECKING:
     from typing_extensions import TypeGuard
 
+# keep these imports up to date with conditional imports in test_gen_exports
+# isort: split
 import astor
-import attr
 import isort.api
 import isort.exceptions
 
@@ -105,8 +108,12 @@ def create_passthrough_args(funcdef: ast.FunctionDef | ast.AsyncFunctionDef) ->
 def run_linters(file: File, source: str) -> str:
     """Run isort and black on the specified file, returning the new source.
 
-    :raises ValueError: If either failed.
+    :raises ImportError: If black is not installed
+    :raises SystemExit: If either failed.
     """
+    # imported to check that `subprocess` calls to black will succeed
+    import black  # noqa: F401
+
     # Black has an undocumented API, but it doesn't easily allow reading configuration from
     # pyproject.toml, and simultaneously pass in / receive the code as a string.
     # https://github.com/psf/black/issues/779