From 3038d75622d14393f67ad7c16076a42a235b68b6 Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 19 Mar 2024 16:45:19 +0000 Subject: [PATCH 1/6] [refactor] Begin reworking the tests.utils HTTP test server interface. --- tests/test_builders/test_build_linkcheck.py | 12 ++++++------ tests/utils.py | 8 ++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/test_builders/test_build_linkcheck.py b/tests/test_builders/test_build_linkcheck.py index c630b700e16..f19838d0504 100644 --- a/tests/test_builders/test_build_linkcheck.py +++ b/tests/test_builders/test_build_linkcheck.py @@ -28,7 +28,7 @@ from sphinx.testing.util import strip_escseq from sphinx.util import requests -from tests.utils import CERT_FILE, http_server, https_server +from tests.utils import CERT_FILE, http_server ts_re = re.compile(r".*\[(?P.*)\].*") @@ -633,7 +633,7 @@ def test_invalid_ssl(get_request, app): @pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-https', freshenv=True) def test_connect_to_selfsigned_fails(app): - with https_server(OKHandler): + with http_server(OKHandler, tls_enabled=True): app.build() with open(app.outdir / 'output.json', encoding='utf-8') as fp: @@ -648,7 +648,7 @@ def test_connect_to_selfsigned_fails(app): @pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-https', freshenv=True) def test_connect_to_selfsigned_with_tls_verify_false(app): app.config.tls_verify = False - with https_server(OKHandler): + with http_server(OKHandler, tls_enabled=True): app.build() with open(app.outdir / 'output.json', encoding='utf-8') as fp: @@ -666,7 +666,7 @@ def test_connect_to_selfsigned_with_tls_verify_false(app): @pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-https', freshenv=True) def test_connect_to_selfsigned_with_tls_cacerts(app): app.config.tls_cacerts = CERT_FILE - with https_server(OKHandler): + with http_server(OKHandler, tls_enabled=True): app.build() with open(app.outdir / 'output.json', encoding='utf-8') as fp: @@ -684,7 +684,7 @@ def test_connect_to_selfsigned_with_tls_cacerts(app): @pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-https', freshenv=True) def test_connect_to_selfsigned_with_requests_env_var(monkeypatch, app): monkeypatch.setenv("REQUESTS_CA_BUNDLE", CERT_FILE) - with https_server(OKHandler): + with http_server(OKHandler, tls_enabled=True): app.build() with open(app.outdir / 'output.json', encoding='utf-8') as fp: @@ -702,7 +702,7 @@ def test_connect_to_selfsigned_with_requests_env_var(monkeypatch, app): @pytest.mark.sphinx('linkcheck', testroot='linkcheck-localserver-https', freshenv=True) def test_connect_to_selfsigned_nonexistent_cert_file(app): app.config.tls_cacerts = "does/not/exist" - with https_server(OKHandler): + with http_server(OKHandler, tls_enabled=True): app.build() with open(app.outdir / 'output.json', encoding='utf-8') as fp: diff --git a/tests/utils.py b/tests/utils.py index 1fbd431ebec..95a6d4e6baf 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -68,5 +68,9 @@ def server(handler_class: type[BaseRequestHandler]) -> Generator[_T_co, None, No return server -http_server = create_server(HttpServerThread) -https_server = create_server(HttpsServerThread) +def http_server(handler: type[BaseRequestHandler], tls_enabled: bool = False) -> AbstractContextManager[_T_co]: + server_cls = HttpsServerThread if tls_enabled else HttpServerThread + return create_server(server_cls)(handler) + + +__all__ = ["http_server"] From 3c89357a709781bb75074a6c1078089c16680b3e Mon Sep 17 00:00:00 2001 From: James Addison Date: Tue, 19 Mar 2024 16:58:15 +0000 Subject: [PATCH 2/6] [tests] utils: refactor type-hint signatures. Ref: discussion at https://github.com/sphinx-doc/sphinx/pull/12109#discussion_r1527196512 --- tests/utils.py | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 95a6d4e6baf..e81e5449f36 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,17 +1,16 @@ from __future__ import annotations -import contextlib +from contextlib import contextmanager from http.server import ThreadingHTTPServer from pathlib import Path from ssl import PROTOCOL_TLS_SERVER, SSLContext from threading import Thread -from typing import TYPE_CHECKING, TypeVar +from typing import TYPE_CHECKING import filelock if TYPE_CHECKING: - from collections.abc import Callable, Generator - from contextlib import AbstractContextManager + from collections.abc import Iterator from socketserver import BaseRequestHandler from typing import Any, Final @@ -49,28 +48,16 @@ def __init__( self.server.socket = sslcontext.wrap_socket(self.server.socket, server_side=True) -_T_co = TypeVar('_T_co', bound=HttpServerThread, covariant=True) - - -def create_server( - server_thread_class: type[_T_co], -) -> Callable[[type[BaseRequestHandler]], AbstractContextManager[_T_co]]: - @contextlib.contextmanager - def server(handler_class: type[BaseRequestHandler]) -> Generator[_T_co, None, None]: - lock = filelock.FileLock(LOCK_PATH) - with lock: - server_thread = server_thread_class(handler_class, daemon=True) - server_thread.start() - try: - yield server_thread - finally: - server_thread.terminate() - return server - - -def http_server(handler: type[BaseRequestHandler], tls_enabled: bool = False) -> AbstractContextManager[_T_co]: +@contextmanager +def http_server(handler: type[BaseRequestHandler], tls_enabled: bool = False) -> Iterator[HttpServerThread]: server_cls = HttpsServerThread if tls_enabled else HttpServerThread - return create_server(server_cls)(handler) + with filelock.FileLock(LOCK_PATH): + server = server_cls(handler, daemon=True) + server.start() + try: + yield server + finally: + server.terminate() __all__ = ["http_server"] From 643cfb0fb111db45b3b640fc5b3a6ae7d02b4cf9 Mon Sep 17 00:00:00 2001 From: James Addison <55152140+jayaddison@users.noreply.github.com> Date: Wed, 20 Mar 2024 10:25:49 +0000 Subject: [PATCH 3/6] [tests] utils: make http_server 'tls_enabled' parameter keyword-only. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index e81e5449f36..996aa1db5df 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -49,7 +49,7 @@ def __init__( @contextmanager -def http_server(handler: type[BaseRequestHandler], tls_enabled: bool = False) -> Iterator[HttpServerThread]: +def http_server(handler: type[BaseRequestHandler], *, tls_enabled: bool = False) -> Iterator[HttpServerThread]: server_cls = HttpsServerThread if tls_enabled else HttpServerThread with filelock.FileLock(LOCK_PATH): server = server_cls(handler, daemon=True) From 31d79df58f6a881ae8ffbfe847089de5fa2d9fbd Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 20 Mar 2024 10:29:28 +0000 Subject: [PATCH 4/6] [tests] utils: move module-level attribute below __future__ import, before other imports (PEP8) --- tests/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 996aa1db5df..63862667b67 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,5 +1,8 @@ from __future__ import annotations +__all__ = ["http_server"] + + from contextlib import contextmanager from http.server import ThreadingHTTPServer from pathlib import Path @@ -58,6 +61,3 @@ def http_server(handler: type[BaseRequestHandler], *, tls_enabled: bool = False) yield server finally: server.terminate() - - -__all__ = ["http_server"] From 4498a4ac3ac56c4115e086df6db8fb8873a3462a Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 20 Mar 2024 13:50:03 +0000 Subject: [PATCH 5/6] [codestyle] test utils: fixup: single line both before-and-after module-level dunder functions. --- tests/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 63862667b67..57cb8e17af6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,7 +2,6 @@ __all__ = ["http_server"] - from contextlib import contextmanager from http.server import ThreadingHTTPServer from pathlib import Path From 0cbadaa0f7d6533c0db0e4e8dff93ed508a70056 Mon Sep 17 00:00:00 2001 From: James Addison Date: Wed, 20 Mar 2024 15:21:00 +0000 Subject: [PATCH 6/6] [tests] use a tuple type for the utils module __all__ module declaration. Ref: https://github.com/python/typing/blob/9a39406fa45fbfa76eb5d35afb7863f5f727ad87/docs/source/libraries.rst?plain=1#L108-L119 Co-authored-by: Chris Sewell --- tests/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 57cb8e17af6..6e3aed2c9cf 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,6 +1,6 @@ from __future__ import annotations -__all__ = ["http_server"] +__all__ = ("http_server",) from contextlib import contextmanager from http.server import ThreadingHTTPServer