refactor proxy tests

getsentry · Feb 5, 2025 · b9f07cd · b9f07cd
1 parent 234661c
commit b9f07cd
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 160 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 **Features**:
 
-- Honor `http(s)_proxy` environment variables on macOS crashpad transport. ([#1111](https://github.com/getsentry/sentry-native/pull/1111))
+- Honor `https_proxy` environment variable on crashpad transport. ([#1111](https://github.com/getsentry/sentry-native/pull/1111))
 - Auto-detect the latest GDK and Windows SDK for the XBox build. ([#1124](https://github.com/getsentry/sentry-native/pull/1124))
 - Enable debug-option by default when running in a debug-build. ([#1128](https://github.com/getsentry/sentry-native/pull/1128))
 

diff --git a/tests/__init__.py b/tests/__init__.py
@@ -8,13 +8,10 @@
 import pytest
 import pprint
 import textwrap
-import time
 import socket
 
-
 sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 
-
 # https://docs.pytest.org/en/latest/assert.html#assert-details
 pytest.register_assert_rewrite("tests.assertions")
 from tests.assertions import assert_no_proxy_request
@@ -31,7 +28,7 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False):
         # to the hosts file & make the DSN using this alternate hostname
         # see https://learn.microsoft.com/en-us/windows/win32/wininet/enabling-internet-functionality#listing-the-proxy-bypass
         host = host.replace("127.0.0.1", "sentry.native.test")
-        check_sentry_native_resolves_to_localhost()
+        _check_sentry_native_resolves_to_localhost()
 
     return urllib.parse.urlunsplit(
         (
@@ -44,68 +41,14 @@ def make_dsn(httpserver, auth="uiaeosnrtdy", id=123456, proxy_host=False):
     )
 
 
-def check_sentry_native_resolves_to_localhost():
+def _check_sentry_native_resolves_to_localhost():
     try:
         resolved_ip = socket.gethostbyname("sentry.native.test")
         assert resolved_ip == "127.0.0.1"
     except socket.gaierror:
         pytest.skip("sentry.native.test does not resolve to localhost")
 
 
-def is_proxy_running(host, port):
-    try:
-        with socket.create_connection((host, port), timeout=1):
-            return True
-    except ConnectionRefusedError:
-        return False
-
-
-def start_mitmdump(proxy_type, proxy_auth: str = None):
-    # start mitmdump from terminal
-    if proxy_type == "http-proxy":
-        proxy_command = ["mitmdump"]
-        if proxy_auth:
-            proxy_command += ["-v", "--proxyauth", proxy_auth]
-        proxy_process = subprocess.Popen(
-            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
-        )
-        time.sleep(5)  # Give mitmdump some time to start
-        if not is_proxy_running("localhost", 8080):
-            pytest.fail("mitmdump (HTTP) did not start correctly")
-    elif proxy_type == "socks5-proxy":
-        proxy_command = ["mitmdump", "--mode", "socks5"]
-        if proxy_auth:
-            proxy_command += ["-v", "--proxyauth", proxy_auth]
-        proxy_process = subprocess.Popen(
-            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
-        )
-        time.sleep(5)  # Give mitmdump some time to start
-        if not is_proxy_running("localhost", 1080):
-            pytest.fail("mitmdump (SOCKS5) did not start correctly")
-    return proxy_process
-
-
-def proxy_test_finally(
-    expected_logsize,
-    httpserver,
-    proxy_process,
-    proxy_log_assert=assert_no_proxy_request,
-):
-    if proxy_process:
-        # Give mitmdump some time to get a response from the mock server
-        time.sleep(0.5)
-        proxy_process.terminate()
-        proxy_process.wait()
-        stdout, stderr = proxy_process.communicate()
-        if expected_logsize == 0:
-            # don't expect any incoming requests to go through the proxy
-            proxy_log_assert(stdout)
-        else:
-            # request passed through successfully
-            assert "POST" in stdout and "200 OK" in stdout
-    assert len(httpserver.log) == expected_logsize
-
-
 def run(cwd, exe, args, env=dict(os.environ), **kwargs):
     __tracebackhide__ = True
     if os.environ.get("ANDROID_API"):

diff --git a/tests/proxy.py b/tests/proxy.py
@@ -0,0 +1,73 @@
+import os
+import socket
+import subprocess
+import time
+
+import pytest
+
+from tests import assert_no_proxy_request
+
+
+def setup_proxy_env_vars(port):
+    os.environ["http_proxy"] = f"http://localhost:{port}"
+    os.environ["https_proxy"] = f"http://localhost:{port}"
+
+
+def cleanup_proxy_env_vars():
+    del os.environ["http_proxy"]
+    del os.environ["https_proxy"]
+
+
+def is_proxy_running(host, port):
+    try:
+        with socket.create_connection((host, port), timeout=1):
+            return True
+    except ConnectionRefusedError:
+        return False
+
+
+def start_mitmdump(proxy_type, proxy_auth: str = None):
+    # start mitmdump from terminal
+    proxy_process = None
+    if proxy_type == "http-proxy":
+        proxy_command = ["mitmdump"]
+        if proxy_auth:
+            proxy_command += ["-v", "--proxyauth", proxy_auth]
+        proxy_process = subprocess.Popen(
+            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+        )
+        time.sleep(5)  # Give mitmdump some time to start
+        if not is_proxy_running("localhost", 8080):
+            pytest.fail("mitmdump (HTTP) did not start correctly")
+    elif proxy_type == "socks5-proxy":
+        proxy_command = ["mitmdump", "--mode", "socks5"]
+        if proxy_auth:
+            proxy_command += ["-v", "--proxyauth", proxy_auth]
+        proxy_process = subprocess.Popen(
+            proxy_command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True
+        )
+        time.sleep(5)  # Give mitmdump some time to start
+        if not is_proxy_running("localhost", 1080):
+            pytest.fail("mitmdump (SOCKS5) did not start correctly")
+    return proxy_process
+
+
+def proxy_test_finally(
+    expected_logsize,
+    httpserver,
+    proxy_process,
+    proxy_log_assert=assert_no_proxy_request,
+):
+    if proxy_process:
+        # Give mitmdump some time to get a response from the mock server
+        time.sleep(0.5)
+        proxy_process.terminate()
+        proxy_process.wait()
+        stdout, stderr = proxy_process.communicate()
+        if expected_logsize == 0:
+            # don't expect any incoming requests to make it through the proxy
+            proxy_log_assert(stdout)
+        else:
+            # request passed through successfully
+            assert "POST" in stdout and "200 OK" in stdout
+    assert len(httpserver.log) == expected_logsize
diff --git a/tests/test_integration_crashpad.py b/tests/test_integration_crashpad.py
@@ -6,7 +6,17 @@
 
 import pytest
 
-from . import make_dsn, run, Envelope, start_mitmdump, proxy_test_finally
+from . import (
+    make_dsn,
+    run,
+    Envelope,
+)
+from .proxy import (
+    setup_proxy_env_vars,
+    cleanup_proxy_env_vars,
+    start_mitmdump,
+    proxy_test_finally,
+)
 from .assertions import (
     assert_crashpad_upload,
     assert_session,
@@ -39,47 +49,58 @@ def test_crashpad_capture(cmake, httpserver):
     assert len(httpserver.log) == 2
 
 
-@pytest.mark.parametrize("port_correct", [True, False])  # TODO separate tests
-def test_crashpad_crash_proxy_env(cmake, httpserver, port_correct):
+def _setup_crashpad_proxy_test(cmake, httpserver, proxy):
+    proxy_process = start_mitmdump(proxy) if proxy else None
+
+    tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"})
+
+    # make sure we are isolated from previous runs
+    shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True)
+
+    env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True))
+    httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data("OK")
+
+    return env, proxy_process, tmp_path
+
+
+def test_crashpad_crash_proxy_env(cmake, httpserver):
     if not shutil.which("mitmdump"):
         pytest.skip("mitmdump is not installed")
 
     proxy_process = None  # store the proxy process to terminate it later
-    port = "8080" if port_correct else "8081"
-    os.environ["http_proxy"] = f"http://localhost:{port}"
-    os.environ["https_proxy"] = f"http://localhost:{port}"
-    expected_logsize = 0
+    setup_proxy_env_vars(port=8080)
     try:
-        proxy_process = start_mitmdump("http-proxy")
+        env, proxy_process, tmp_path = _setup_crashpad_proxy_test(
+            cmake, httpserver, "http-proxy"
+        )
+
+        with httpserver.wait(timeout=10) as waiting:
+            child = run(tmp_path, "sentry_example", ["log", "crash"], env=env)
+            assert child.returncode  # well, it's a crash after all
+        assert waiting.result
+    finally:
+        proxy_test_finally(1, httpserver, proxy_process)
+        cleanup_proxy_env_vars()
 
-        tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"})
 
-        # make sure we are isolated from previous runs
-        shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True)
+def test_crashpad_crash_proxy_env_port_incorrect(cmake, httpserver):
+    if not shutil.which("mitmdump"):
+        pytest.skip("mitmdump is not installed")
 
-        env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True))
-        httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data(
-            "OK"
+    proxy_process = None  # store the proxy process to terminate it later
+    setup_proxy_env_vars(port=8081)
+    try:
+        env, proxy_process, tmp_path = _setup_crashpad_proxy_test(
+            cmake, httpserver, "http-proxy"
         )
 
-        try:
-            with httpserver.wait(timeout=10) as waiting:
+        with pytest.raises(AssertionError):
+            with httpserver.wait(timeout=10):
                 child = run(tmp_path, "sentry_example", ["log", "crash"], env=env)
                 assert child.returncode  # well, it's a crash after all
-        except AssertionError as e:
-            if port_correct:
-                raise e
-            else:
-                expected_logsize = 0
-                return
-
-        assert waiting.result
-
-        expected_logsize = 1 if port_correct else 0
     finally:
-        proxy_test_finally(expected_logsize, httpserver, proxy_process)
-        del os.environ["http_proxy"]
-        del os.environ["https_proxy"]
+        proxy_test_finally(0, httpserver, proxy_process)
+        cleanup_proxy_env_vars()
 
 
 @pytest.mark.parametrize(
@@ -104,18 +125,9 @@ def test_crashpad_crash_proxy(cmake, httpserver, run_args, proxy_running):
     expected_logsize = 0
 
     try:
-        if proxy_running:
-            # start mitmdump from terminal
-            proxy_process = start_mitmdump(run_args[0])
-
-        tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "crashpad"})
-
-        # make sure we are isolated from previous runs
-        shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True)
-
-        env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver, proxy_host=True))
-        httpserver.expect_oneshot_request("/api/123456/minidump/").respond_with_data(
-            "OK"
+        proxy_to_start = run_args[0] if proxy_running else None
+        env, proxy_process, tmp_path = _setup_crashpad_proxy_test(
+            cmake, httpserver, proxy_to_start
         )
 
         try: