From c9446931a80e63ea1d77e130ea5581b547e0f51b Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 9 Jul 2026 20:02:05 +0500 Subject: [PATCH] Use external mitmproxy. (#7720) --- .github/workflows/tests-ubuntu.yml | 6 +- conftest.py | 6 +- docs/contributing.rst | 18 +++++ pyproject.toml | 2 +- tests/mockserver/mitm_proxy.py | 104 ++++++++++++++++++++--------- tests/mockserver/utils.py | 8 +++ tox.ini | 16 +---- 7 files changed, 109 insertions(+), 51 deletions(-) diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index f51dd9799..15f25d9b8 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -79,9 +79,6 @@ jobs: - python-version: "3.14" env: TOXENV: botocore - - python-version: "3.14" - env: - TOXENV: mitmproxy steps: - uses: actions/checkout@v6 @@ -97,6 +94,9 @@ jobs: sudo apt-get update sudo apt-get install libxml2-dev libxslt-dev + - name: Install mitmproxy + run: pipx install mitmproxy + - name: Run tests env: ${{ matrix.env }} run: | diff --git a/conftest.py b/conftest.py index 532f83f56..7403b07b2 100644 --- a/conftest.py +++ b/conftest.py @@ -11,7 +11,7 @@ from scrapy.utils.reactor import set_asyncio_event_loop_policy from scrapy.utils.reactorless import install_reactor_import_hook from tests.keys import generate_keys from tests.mockserver.http import MockServer -from tests.mockserver.mitm_proxy import MitmProxy +from tests.mockserver.mitm_proxy import MitmProxy, mitmdump_cmd if TYPE_CHECKING: from collections.abc import Generator @@ -127,7 +127,6 @@ def pytest_runtest_setup(item): "uvloop", "botocore", "boto3", - "mitmproxy", ] for module in optional_deps: @@ -137,6 +136,9 @@ def pytest_runtest_setup(item): except ImportError: pytest.skip(f"{module} is not installed") + if item.get_closest_marker("requires_mitmproxy") and mitmdump_cmd() is None: + pytest.skip("mitmdump is not available") + # Generate localhost certificate files, needed by some tests generate_keys() diff --git a/docs/contributing.rst b/docs/contributing.rst index b43d30796..6d2e08fe8 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -372,6 +372,21 @@ To see coverage report install :doc:`coverage ` see output of ``coverage --help`` for more options like html or xml report. +Some tests need a ``mitmdump`` executable (from mitmproxy_) to test against a +fully featured proxy server; they are skipped when one cannot be found +(``mitmproxy`` is intentionally not a test dependency that would be installed +into test venvs, as that sometimes leads to various dependency conflicts). +To run these tests, make ``mitmdump`` available in one of these ways: + +* install ``mitmproxy`` so that ``mitmdump`` is on your ``PATH``, e.g. with + pipx_ (``pipx install mitmproxy``) or uv_ (``uv tool install mitmproxy``); + +* have uv_ installed, in which case the tests will run + ``uvx --from mitmproxy mitmdump``; + +* set the ``MITMDUMP`` environment variable to the path of a ``mitmdump`` + executable. + Writing tests ------------- @@ -399,3 +414,6 @@ And their unit-tests are in:: .. _pytest-xdist: https://github.com/pytest-dev/pytest-xdist .. _help wanted issues: https://github.com/scrapy/scrapy/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22 .. _test coverage: https://app.codecov.io/gh/scrapy/scrapy +.. _mitmproxy: https://mitmproxy.org/ +.. _pipx: https://pipx.pypa.io/ +.. _uv: https://docs.astral.sh/uv/ diff --git a/pyproject.toml b/pyproject.toml index ac9d37e78..b24397c02 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -344,7 +344,7 @@ markers = [ "requires_uvloop: marks tests as only enabled when uvloop is known to be working", "requires_botocore: marks tests that need botocore (but not boto3)", "requires_boto3: marks tests that need botocore and boto3", - "requires_mitmproxy: marks tests that need mitmproxy", + "requires_mitmproxy: marks tests that need a mitmdump executable", "requires_internet: marks tests that need real Internet access", ] filterwarnings = [ diff --git a/tests/mockserver/mitm_proxy.py b/tests/mockserver/mitm_proxy.py index 56620f84e..8a1025b37 100644 --- a/tests/mockserver/mitm_proxy.py +++ b/tests/mockserver/mitm_proxy.py @@ -1,11 +1,42 @@ from __future__ import annotations -import re -import sys +import contextlib +import functools +import os +import shutil +import signal +import socket +import time from pathlib import Path -from subprocess import PIPE, Popen +from subprocess import DEVNULL, Popen from urllib.parse import urlsplit, urlunsplit +from .utils import _free_port + + +@functools.cache +def mitmdump_cmd() -> list[str] | None: + """Return the command prefix used to invoke ``mitmdump``, or ``None`` if it + cannot be resolved. + + We don't want to install ``mitmproxy`` into the test env (it has a lot of + dependencies that can conflict with some of the Scrapy/test ones, and its + newer versions may not support older Python versions). So we expect it + installed externally. We look for the ``mitmdump`` binary in the following + sources: + + 1. the ``MITMDUMP`` environment variable; + 2. a ``mitmdump`` binary on ``PATH``; + 3. using ``uvx --from mitmproxy mitmdump`` if ``uvx`` is available. + """ + if env := os.environ.get("MITMDUMP"): + return [env] + if path := shutil.which("mitmdump"): + return [path] + if uvx := shutil.which("uvx"): + return [uvx, "--from", "mitmproxy", "mitmdump"] + return None + class MitmProxy: auth_user = "scrapy" @@ -15,18 +46,22 @@ class MitmProxy: self.mode = mode def start(self) -> str: - script = """ -import sys -from mitmproxy.tools.main import mitmdump -sys.argv[0] = "mitmdump" -sys.exit(mitmdump()) - """ + cmd = mitmdump_cmd() + if not cmd: + raise RuntimeError( + "mitmdump is not available. Please install mitmproxy or uv." + ) cert_path = Path(__file__).parent.parent.resolve() / "keys" + # Choose a free port ourselves instead of reading the mitmdump output + # as there is no easy way to disable stdout buffering for all kinds of + # mitmdump installs that we support. + host = "127.0.0.1" + port = _free_port() args = [ "--listen-host", - "127.0.0.1", + host, "--listen-port", - "0", + str(port), "--proxyauth", f"{self.auth_user}:{self.auth_pass}", "--set", @@ -37,30 +72,39 @@ sys.exit(mitmdump()) ] if self.mode: args += ["--mode", self.mode] - self.proc: Popen[str] = Popen( - [ - sys.executable, - "-u", - "-c", - script, - *args, - ], - stdout=PIPE, - text=True, + self.proc: Popen[bytes] = Popen( + [*cmd, *args], + stdout=DEVNULL, + stderr=DEVNULL, + start_new_session=True, # needed for killpg() to make sense ) - assert self.proc.stdout is not None scheme = "socks5" if self.mode == "socks5" else "http" - line = "" - for line in self.proc.stdout: - m = re.search(r"listening at (?:\w+://)?([^:]+:\d+)", line) - if m: - host_port = m.group(1) - return f"{scheme}://{self.auth_user}:{self.auth_pass}@{host_port}" + deadline = time.monotonic() + 60 + while True: + if self.proc.poll() is not None: + raise RuntimeError( + f"mitmdump exited with code {self.proc.returncode} before it " + f"started listening" + ) + try: + with socket.create_connection((host, port), timeout=1): + return f"{scheme}://{self.auth_user}:{self.auth_pass}@{host}:{port}" + except OSError: + if time.monotonic() >= deadline: + break + time.sleep(0.05) self.stop() - raise RuntimeError(f"Failed to parse mitmdump output: {line}") + raise RuntimeError(f"mitmdump did not start listening on {host}:{port} in time") def stop(self) -> None: - self.proc.kill() + if os.name == "posix": + # SIGKILL doesn't propagate to the actual process (child of uvx) + # https://github.com/astral-sh/uv/issues/11817#issuecomment-2688830077 + # https://stackoverflow.com/a/61980200/113586 + with contextlib.suppress(ProcessLookupError): + os.killpg(os.getpgid(self.proc.pid), signal.SIGKILL) + else: + self.proc.kill() self.proc.communicate() diff --git a/tests/mockserver/utils.py b/tests/mockserver/utils.py index 5c4ca7457..33c3340b9 100644 --- a/tests/mockserver/utils.py +++ b/tests/mockserver/utils.py @@ -1,5 +1,6 @@ from __future__ import annotations +import socket from pathlib import Path from typing import TYPE_CHECKING, cast @@ -18,6 +19,13 @@ if TYPE_CHECKING: from twisted.internet.interfaces import IOpenSSLContextFactory +def _free_port() -> int: + # racy but should be fine for tests + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return cast("int", s.getsockname()[1]) + + def ssl_context_factory( keyfile: str = "keys/localhost.key", certfile: str = "keys/localhost.crt", diff --git a/tox.ini b/tox.ini index 7ace26e98..526533480 100644 --- a/tox.ini +++ b/tox.ini @@ -28,7 +28,6 @@ envlist = no-reactor no-reactor-extra-deps botocore - mitmproxy pypy3 pypy3-extra-deps minversion = 1.7.0 @@ -55,6 +54,7 @@ deps = passenv = PYTHONTRACEMALLOC PYTEST_ADDOPTS + MITMDUMP S3_TEST_FILE_URI AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY @@ -319,17 +319,3 @@ setenv = {[min]setenv} commands = pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=min-botocore.junit.xml -o junit_family=legacy} -m requires_botocore - - -# Run proxy tests that use mitmproxy in a separate env to avoid installing -# numerous mitmproxy deps in other envs (even in extra-deps), as they can -# conflict with other deps we want, or don't want, to have installed there. - -[testenv:mitmproxy] -deps = - {[testenv]deps} - # mitmproxy does not support PyPy - mitmproxy; implementation_name != "pypy" - httpx[http2,socks] -commands = - pytest {posargs:--cov-config=pyproject.toml --cov=scrapy --cov-report=xml --cov-report= tests --junitxml=mitmproxy.junit.xml -o junit_family=legacy} -m requires_mitmproxy