tests: add --strict-optional flag for dependency-gated skips

.gitignore

@@ -389,6 +389,8 @@ Release
 test/__pycache__
 test/*/__pycache__
 test/*/*/__pycache__
+__pycache__/
+*.pyc
 
 # make check
 check

test/README.pytest

@@ -93,6 +93,52 @@ For example, most tests in test/modules/md require an installation
 of 'pebble', an ACME test server, and look for it in $PATH.
 
 
+Strict optional dependencies
+----------------------------
+A skip is convenient locally and dangerous in CI: a run that was meant
+to cover SSL, HTTP/2 and mod_md stays green while quietly covering none
+of them, because the machine was misconfigured. Strict mode turns that
+silence into a failure.
+
+> pytest --strict-optional
+> PYHTTPD_STRICT_OPTIONAL=1 pytest
+
+Both are off by default, so nothing changes unless you ask for it. The
+env var is the one to set in CI; the flag is the one to use by hand.
+
+Not every skip becomes a failure. Skips come in two kinds:
+
+  - "does not apply here" -- wrong MPM, httpd too old, STRESS_TEST not
+    set, $SSL selects a different TLS module. These always skip, in
+    strict mode too, and a test gated on both kinds skips rather than
+    fails.
+
+  - "something we expected to be installed is missing" -- curl, nghttp,
+    h2load, the 'websockets' and 'multipart' python packages, mod_ssl,
+    mod_tls, mod_md, a2md, the ACME test server, an OCSP responder.
+    These fail under strict mode.
+
+The OCSP responder is the one judgement call in that split. pebble
+serves no OCSP, boulder does, so with ACME=pebble the stapling tests
+have nothing to talk to. That is a property of the environment rather
+than of this machine's packages, and a pipeline that is meant to cover
+OCSP stapling and runs pebble has lost that coverage silently -- which
+is the thing strict mode exists to catch. So it fails, and the message
+names ACME=pebble as what was detected. Run ACME=boulder to cover
+stapling, or leave strict mode off.
+
+A strict failure is raised during test setup, so pytest reports it as
+ERROR rather than FAILED; the exit code is non-zero either way. The
+message is prefixed '[strict-optional]' so it greps cleanly out of a CI
+log, and names the dependency, what was expected, what was found
+instead, and how to switch strict mode back off.
+
+curl is the one dependency gated for the suite as a whole rather than
+test by test: apache_restart() proves the server came up by making a
+curl request, so without curl nothing under test/modules/ can run. That
+gate lives in the root conftest.py.
+
+
 Workings
 --------
 All tests start httpd on their own and try hard to shut it down
@@ -168,6 +214,39 @@ simplest example. 'test/modules/http2' shows how you load
 other modules. 'test/modules/md' checks and starts external
 processes (an ACME test server).
 
+If your test needs something that may not be installed, say
+so with the decorator from 'pyhttpd.depends'. It takes the
+condition itself, so it works on a method, a class or a
+module-level 'pytestmark':
+
+    from pyhttpd.depends import needs_dependency
+
+    @needs_dependency("a2md", MDTestEnv.has_a2md(),
+                      reason="no a2md available")
+    class TestStore:
+
+For the tools the suite gates on most often there are ready
+made versions of that decorator, which probe the tool once
+and cache the answer:
+
+    @H2TestEnv.needs_nghttp()             # and (assets=True)
+    @H2TestEnv.needs_h2load("1.41.0")
+    @HttpdTestEnv.needs_multipart()
+
+For a check that can only be made once the test is running,
+'env.require()' does the same job from inside the body:
+
+    env.require(env.curl_is_at_least('8.0.0'),
+                'need at least curl v8.0.0 for this',
+                dep='curl',
+                detected=f"curl {env.curl_version_str()}")
+
+Both skip normally and fail under strict mode; see "Strict
+optional dependencies" above. When the reason is instead
+"this test does not apply here", keep using a plain
+'pytest.mark.skipif' or 'pytest.skip()' -- those never
+become failures.
+
 
 Infrastructure
 --------------

test/conftest.py

@@ -10,6 +10,15 @@
 sys.path.append(os.path.join(os.path.dirname(__file__), '.'))
 
 from pyhttpd.env import HttpdTestEnv
+from pyhttpd.depends import apply_gate, strict_optional_enabled
+
+# pyhttpd.depends provides --strict-optional and the needs_dependency marker.
+# pytest_plugins is only honoured in the rootdir conftest, and there is no
+# pytest.ini in this tree, so this is the only place it can be declared.
+pytest_plugins = ["pyhttpd.depends"]
+
+# Everything below test/modules/ drives a real httpd.
+MODULES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'modules')
 
 
 def _has_ipv6():
@@ -148,6 +157,29 @@ def pytest_configure(config):
         _generate_config_ini(config)
 
 
+def pytest_collection_modifyitems(config, items):
+    """Gate the integration suite on the clients the framework itself needs.
+
+    curl is not an optional dependency of the individual test cases that
+    call curl_get(): apache_restart() proves the server came up with
+    is_live(), and is_live() is a curl request. Without curl every test
+    that starts an httpd dies in fixture setup, which is neither a skip nor
+    a useful failure. So it is gated once here for the whole of
+    test/modules/ rather than test by test.
+
+    Tests outside that directory - the strict-mode self-test - run no httpd
+    and are left alone.
+    """
+    env = HttpdTestEnv()
+    if env.has_curl():
+        return
+    missing = [("curl", "no curl available", f"tried to run {env.curl}")]
+    strict = strict_optional_enabled(config)
+    for item in items:
+        if str(item.path).startswith(MODULES_DIR):
+            apply_gate(item, missing, strict)
+
+
 def pytest_generate_tests(metafunc):
     if "repeat" in metafunc.fixturenames:
         count = int(metafunc.config.getoption("repeat"))
@@ -199,6 +231,6 @@ def _package_scope(env, request):
         warnings.warn("--archive option was empty, skipping archiving")
     if archive_dir:
         fspath = str(request.fspath)
-        parts = fspath.split('modules/')
-        package_name = parts[1].split('/')[0]
-        env.archive_logs(package_name, archive_dir)
+        if 'modules/' in fspath:
+            package_name = fspath.split('modules/')[1].split('/')[0]
+            env.archive_logs(package_name, archive_dir)

test/modules/arch/linux/conftest.py

@@ -24,8 +24,9 @@ def env(pytestconfig) -> SystemdTestEnv:
     logging.getLogger('').addHandler(console)
     logging.getLogger('').setLevel(level=level)
     env = SystemdTestEnv(pytestconfig=pytestconfig)
-    if not env.has_systemd_module:
-        pytest.skip("mod_systemd is not built, configure with --enable-systemd")
+    env.require(env.has_systemd_module, "mod_systemd is not built, configure with --enable-systemd",
+                dep="mod_systemd",
+                detected=f"not found in {env.libexec_dir}")
     env.setup_httpd()
     env.apache_access_log_clear()
     env.httpd_error_log.clear_log()

test/modules/arch/linux/test_005_service.py

@@ -2,11 +2,14 @@
 
 import pytest
 
+from pyhttpd.depends import needs_dependency
+
 from .env import MONITOR_STATUS, MONITOR_TIMEOUT, TransientService, http_responds
 
-pytestmark = pytest.mark.skipif(
-    not TransientService.is_available(),
-    reason="no per-user systemd manager to run a transient service under")
+pytestmark = needs_dependency(
+    "systemd --user manager", TransientService.is_available(),
+    reason="no per-user systemd manager to run a transient service under",
+    detected="systemd-run --user is not usable here")
 
 
 class TestSystemdService:

test/modules/arch/linux/test_006_watchdog.py

@@ -4,6 +4,8 @@
 
 import pytest
 
+from pyhttpd.depends import needs_dependency
+
 from .env import (ForegroundServer, TransientService, MIN_WATCHDOG_SEC,
                   NO_MONITOR_TIMEOUT, WATCHDOG_TIMEOUT, is_watchdog_ping)
 
@@ -187,8 +189,9 @@ def test_systemd_006_09_workable_timeout_not_warned(self, env,
             env.httpd_error_log.scan_recent(
                 re.compile(r'.*AH10621: .*'), timeout=1)
 
-    @pytest.mark.skipif(not TransientService.is_available(),
-                        reason="no per-user systemd manager")
+    @needs_dependency("systemd --user manager", TransientService.is_available(),
+                      reason="no per-user systemd manager",
+                      detected="systemd-run --user is not usable here")
     def test_systemd_006_10_service_keeps_watchdog_alive(self, env):
         """The real thing: systemd records each keep-alive it receives, and
         the unit stays active rather than failing with Result=watchdog."""

test/modules/arch/linux/test_007_notify_reload.py

@@ -3,13 +3,17 @@
 
 import pytest
 
+from pyhttpd.depends import needs_dependency
+
 from .env import (NOTIFY_RELOAD_VERSION, TransientService, error_log_since,
                   error_log_size, http_responds, systemd_version)
 
 pytestmark = [
-    pytest.mark.skipif(
-        not TransientService.is_available(),
-        reason="no per-user systemd manager to run a transient service under"),
+    needs_dependency(
+        "systemd --user manager", TransientService.is_available(),
+        reason="no per-user systemd manager to run a transient service under",
+        detected="systemd-run --user is not usable here"),
+    # a version gate, not a missing dependency: keeps skipping when strict
     pytest.mark.skipif(
         systemd_version() < NOTIFY_RELOAD_VERSION,
         reason=f"Type=notify-reload needs systemd {NOTIFY_RELOAD_VERSION}"),

test/modules/core/test_002_restarts.py

@@ -88,8 +88,8 @@ def _rtime(self, s: str) -> timedelta:
 
 @pytest.mark.skipif(condition='STRESS_TEST' not in os.environ,
                     reason="STRESS_TEST not set in env")
-@pytest.mark.skipif(condition=not CoreTestEnv().h2load_is_at_least('1.41.0'),
-                    reason="h2load unavailable or misses --connect-to option")
+@CoreTestEnv.needs_h2load("1.41.0",
+                          reason="h2load unavailable or misses --connect-to option")
 class TestRestarts:
 
     def test_core_002_01(self, env):

test/modules/generators/test_001_cgid.py

@@ -18,8 +18,9 @@ class TestCgid:
 
     @pytest.fixture(autouse=True, scope='class')
     def _class_scope(self, env):
-        if not env.has_cgid_module:
-            pytest.skip("mod_cgid not built")
+        env.require(env.has_cgid_module, "mod_cgid not built",
+                    dep="mod_cgid",
+                    detected=f"not found in {env.libexec_dir}")
         conf = HttpdConf(env, extras={
             'base': textwrap.dedent(f"""
             LogLevel cgid:trace1

test/modules/http1/test_004_post.py

@@ -7,9 +7,12 @@
 
 import pytest
 
+from pyhttpd.env import HttpdTestEnv
+
 from .env import H1Conf
 
 
+@HttpdTestEnv.needs_multipart()
 class TestPost:
 
     @pytest.fixture(autouse=True, scope='class')

test/modules/http1/test_005_trailers.py

@@ -1,11 +1,13 @@
 import os
 import pytest
 
+from pyhttpd.env import HttpdTestEnv
+
 from .env import H1Conf
 
 
-# The trailer tests depend on "nghttp" as no other client seems to be able to send those
-# rare things.
+# These upload to the cgi handler, which parses the multipart body.
+@HttpdTestEnv.needs_multipart()
 class TestTrailers:
 
     @pytest.fixture(autouse=True, scope='class')

test/modules/http2/conftest.py

@@ -23,6 +23,10 @@ def env(pytestconfig) -> H2TestEnv:
     logging.getLogger('').addHandler(console)
     logging.getLogger('').setLevel(level=level)
     env = H2TestEnv(pytestconfig=pytestconfig)
+    # the h2 vhosts are TLS, so setup_httpd() hard-requires mod_ssl and
+    # raises if it was not built. Report that as the missing dependency.
+    env.require(env.has_mod_ssl(), "no mod_ssl available", dep="mod_ssl",
+                detected=f"no mod_ssl.so in {env.libexec_dir}")
     env.setup_httpd()
     env.apache_access_log_clear()
     env.httpd_error_log.clear_log()

test/modules/http2/htdocs/cgi/echo.py

@@ -1,6 +1,5 @@
 #!/usr/bin/env python3
 import os, sys
-import multipart
 
 status = '200 Ok'
 

test/modules/http2/test_003_get.py

@@ -5,7 +5,7 @@
 
 
 @pytest.mark.skipif(condition=H2TestEnv.is_unsupported(), reason="mod_http2 not supported here")
-@pytest.mark.skipif(not H2TestEnv().has_nghttp(), reason="nghttp not available")
+@H2TestEnv.needs_nghttp()
 class TestGet:
 
     @pytest.fixture(autouse=True, scope='class')

test/modules/http2/test_004_post.py

@@ -13,7 +13,8 @@
 
 
 @pytest.mark.skipif(condition=H2TestEnv.is_unsupported(), reason="mod_http2 not supported here")
-@pytest.mark.skipif(not H2TestEnv().has_nghttp(), reason="nghttp not available")
+@H2TestEnv.needs_nghttp()
+@H2TestEnv.needs_multipart()
 class TestPost:
 
     @pytest.fixture(autouse=True, scope='class')

test/modules/http2/test_006_assets.py

@@ -4,7 +4,7 @@
 
 
 @pytest.mark.skipif(condition=H2TestEnv.is_unsupported(), reason="mod_http2 not supported here")
-@pytest.mark.skipif(not H2TestEnv().has_nghttp(), reason="nghttp not available")
+@H2TestEnv.needs_nghttp(assets=True)
 class TestAssets:
 
     @pytest.fixture(autouse=True, scope='class')

test/modules/http2/test_009_timing.py

@@ -7,7 +7,7 @@
 
 
 @pytest.mark.skipif(condition=H2TestEnv.is_unsupported(), reason="mod_http2 not supported here")
-@pytest.mark.skipif(not H2TestEnv().h2load_is_at_least('1.41.0'), reason="h2load misses --connect-to option")
+@H2TestEnv.needs_h2load("1.41.0", reason="h2load misses --connect-to option")
 class TestTiming:
 
     LOGFILE = ""

test/modules/http2/test_101_ssl_reneg.py

@@ -65,8 +65,9 @@ def test_h2_101_01(self, env):
 
     # try to renegotiate the cipher, should fail with correct code
     def test_h2_101_02(self, env):
-        if not (env.curl_is_at_least('8.2.0') or env.curl_is_less_than('8.1.0')):
-            pytest.skip("need curl != 8.1.x version")
+        env.require(not env.curl_is_8_1_x(), "need curl != 8.1.x version",
+                    dep="curl != 8.1.x",
+                    detected=f"curl {env.curl_version_str()}")
         url = env.mkurl("https", "ssl", "/renegotiate/cipher/")
         r = env.curl_get(url, options=[
             "-vvv", "--tlsv1.2", "--tls-max", "1.2", "--ciphers", "ECDHE-RSA-AES256-GCM-SHA384"
@@ -88,8 +89,9 @@ def test_h2_101_02(self, env):
     # try to renegotiate a client certificate from Location 
     # needs to fail with correct code
     def test_h2_101_03(self, env):
-        if not (env.curl_is_at_least('8.2.0') or env.curl_is_less_than('8.1.0')):
-            pytest.skip("need curl != 8.1.x version")
+        env.require(not env.curl_is_8_1_x(), "need curl != 8.1.x version",
+                    dep="curl != 8.1.x",
+                    detected=f"curl {env.curl_version_str()}")
         url = env.mkurl("https", "ssl", "/renegotiate/verify/")
         r = env.curl_get(url, options=["-vvv", "--tlsv1.2", "--tls-max", "1.2"])
         assert 0 != r.exit_code
@@ -109,8 +111,9 @@ def test_h2_101_03(self, env):
     # try to renegotiate a client certificate from Directory 
     # needs to fail with correct code
     def test_h2_101_04(self, env):
-        if not (env.curl_is_at_least('8.2.0') or env.curl_is_less_than('8.1.0')):
-            pytest.skip("need curl != 8.1.x version")
+        env.require(not env.curl_is_8_1_x(), "need curl != 8.1.x version",
+                    dep="curl != 8.1.x",
+                    detected=f"curl {env.curl_version_str()}")
         url = env.mkurl("https", "ssl", "/ssl-client-verify/index.html")
         r = env.curl_get(url, options=["-vvv", "--tlsv1.2", "--tls-max", "1.2"])
         assert 0 != r.exit_code, f"{r}"
@@ -130,8 +133,8 @@ def test_h2_101_04(self, env):
     # make 10 requests on the same connection, none should produce a status code
     # reported by erki@example.ee
     def test_h2_101_05(self, env):
-        if not env.has_h2load():
-            pytest.skip("h2load not available")
+        env.require(env.has_h2load(), "h2load not available", dep="h2load",
+                    detected=f"tried to run {env.h2load}")
         r = env.run([env.h2load, "-n", "10", "-c", "1", "-m", "1", "-vvvv",
                      f"{env.https_base_url}/ssl-client-verify/index.html"])
         assert 0 == r.exit_code
@@ -165,8 +168,9 @@ def test_h2_101_10b(self, env):
 
     # Check that status works with ErrorDoc, see pull #174, fixes #172
     def test_h2_101_11(self, env):
-        if not (env.curl_is_at_least('8.2.0') or env.curl_is_less_than('8.1.0')):
-            pytest.skip("need curl != 8.1.x version")
+        env.require(not env.curl_is_8_1_x(), "need curl != 8.1.x version",
+                    dep="curl != 8.1.x",
+                    detected=f"curl {env.curl_version_str()}")
         url = env.mkurl("https", "ssl", "/renegotiate/err-doc-cipher")
         r = env.curl_get(url, options=[
             "-vvv", "--tlsv1.2", "--tls-max", "1.2", "--ciphers", "ECDHE-RSA-AES256-GCM-SHA384"