From ef5bb1ade60aed66aaccb776a0fc9eb16d58bb5a Mon Sep 17 00:00:00 2001
From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com>
Date: Fri, 23 Feb 2024 11:21:37 +1100
Subject: [PATCH] Fix httpx incorrectly named method on interceptor subclass
 (#126)

* Use pytest-httpbin to avoid outbound requests in most tests

* Enable aiohttp tests in py3.12

* Add standard tests for all engines

Reproduce https://github.com/h2non/pook/issues/125 in tests

* Fix https://github.com/h2non/pook/issues/125

* Bump to 1.4.3
---
 History.rst                             |  5 +++
 pyproject.toml                          |  5 ++-
 src/pook/__init__.py                    |  2 +-
 src/pook/interceptors/_httpx.py         |  4 +-
 src/pook/interceptors/base.py           |  8 ++--
 tests/unit/interceptors/aiohttp_test.py | 49 ++++++++++++---------
 tests/unit/interceptors/base.py         | 58 +++++++++++++++++++++++++
 tests/unit/interceptors/httpx_test.py   | 44 ++++++++++++-------
 tests/unit/interceptors/module_test.py  |  6 ++-
 tests/unit/interceptors/urllib3_test.py | 41 ++++++++++-------
 tests/unit/interceptors/urllib_test.py  | 19 +++++++-
 tests/unit/matchers/query_test.py       | 17 +++++---
 tests/unit/mock_engine_test.py          | 11 +++--
 13 files changed, 195 insertions(+), 74 deletions(-)
 create mode 100644 tests/unit/interceptors/base.py

diff --git a/History.rst b/History.rst
index 40b2c41..45a59da 100644
--- a/History.rst
+++ b/History.rst
@@ -1,6 +1,11 @@
 History
 =======
 
+v1.4.3 / 2024-02-23
+-------------------
+
+  * Fix httpx incorrectly named method on interceptor subclass by @sarayourfriend in https://github.com/h2non/pook/pull/126
+
 v1.4.2 / 2024-02-15
 -------------------
 
diff --git a/pyproject.toml b/pyproject.toml
index 53e0a56..e1d39cc 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -54,6 +54,7 @@ extra-dependencies = [
     "pytest~=7.4",
     "pytest-asyncio~=0.20.3",
     "pytest-pook==0.1.0b0",
+    "pytest-httpbin==2.0.0",
 
     "requests~=2.20",
     "urllib3~=1.24",
@@ -62,8 +63,8 @@ extra-dependencies = [
     # aiohttp depends on multidict, so we can't test aiohttp until
     # https://github.com/aio-libs/multidict/issues/887 is resolved
     # async-timeout is only used for testing aiohttp
-    "aiohttp~=3.8; python_version < '3.12'",
-    "async-timeout~=4.0.3; python_version < '3.12'",
+    "aiohttp~=3.8",
+    "async-timeout~=4.0.3",
 
     # mocket relies on httptools which does not support PyPy
     "mocket[pook]~=3.12.2; platform_python_implementation != 'PyPy'",
diff --git a/src/pook/__init__.py b/src/pook/__init__.py
index ef3f360..4781a3e 100644
--- a/src/pook/__init__.py
+++ b/src/pook/__init__.py
@@ -9,4 +9,4 @@
 __license__ = "MIT"
 
 # Current version
-__version__ = "1.4.2"
+__version__ = "1.4.3"
diff --git a/src/pook/interceptors/_httpx.py b/src/pook/interceptors/_httpx.py
index 1f11058..2cc07fd 100644
--- a/src/pook/interceptors/_httpx.py
+++ b/src/pook/interceptors/_httpx.py
@@ -42,7 +42,7 @@ def handler(client, *_):
     def activate(self):
         [self._patch(path) for path in PATCHES]
 
-    def deactivate(self):
+    def disable(self):
         [patch.stop() for patch in self.patchers]
 
 
@@ -96,7 +96,7 @@ async def handle_async_request(self, request):
         mock = self._interceptor.engine.match(pook_request)
 
         if not mock:
-            transport = self._original_transport_for_url(self._client, self.request.url)
+            transport = self._original_transport_for_url(self._client, request.url)
             return await transport.handle_async_request(request)
 
         if mock._delay:
diff --git a/src/pook/interceptors/base.py b/src/pook/interceptors/base.py
index 069988a..843b517 100644
--- a/src/pook/interceptors/base.py
+++ b/src/pook/interceptors/base.py
@@ -1,7 +1,7 @@
 from abc import abstractmethod, ABCMeta
 
 
-class BaseInterceptor(object):
+class BaseInterceptor:
     """
     BaseInterceptor provides a base class for HTTP traffic
     interceptors implementations.
@@ -14,7 +14,7 @@ def __init__(self, engine):
         self.engine = engine
 
     @property
-    def name(self):
+    def name(self) -> str:
         """
         Exposes the interceptor class name.
         """
@@ -26,7 +26,7 @@ def activate(self):
         Activates the traffic interceptor.
         This method must be implemented by any interceptor.
         """
-        pass
+        raise NotImplementedError("Sub-classes must implement `activate`")
 
     @abstractmethod
     def disable(self):
@@ -34,4 +34,4 @@ def disable(self):
         Disables the traffic interceptor.
         This method must be implemented by any interceptor.
         """
-        pass
+        raise NotImplementedError("Sub-classes must implement `disable`")
diff --git a/tests/unit/interceptors/aiohttp_test.py b/tests/unit/interceptors/aiohttp_test.py
index a15f580..a29009a 100644
--- a/tests/unit/interceptors/aiohttp_test.py
+++ b/tests/unit/interceptors/aiohttp_test.py
@@ -1,36 +1,41 @@
-import sys
+from pathlib import Path
 
-import pook
 import pytest
+import aiohttp
 
-from pathlib import Path
+import pook
 
-SUPPORTED = sys.version_info < (3, 12)
-if SUPPORTED:
-    # See pyproject.toml comment
-    import aiohttp
+from tests.unit.interceptors.base import StandardTests
 
 
-pytestmark = [
-    pytest.mark.pook,
-    pytest.mark.asyncio,
-    pytest.mark.skipif(
-        not SUPPORTED, reason="See pyproject.toml comment on aiohttp dependency"
-    ),
-]
+pytestmark = [pytest.mark.pook]
 
-URL = "https://httpbin.org/status/404"
+
+class TestStandardAiohttp(StandardTests):
+    is_async = True
+
+    async def amake_request(self, method, url):
+        async with aiohttp.ClientSession(loop=self.loop) as session:
+            req = await session.request(method=method, url=url)
+            content = await req.read()
+            return req.status, content.decode("utf-8")
 
 
 binary_file = (Path(__file__).parents[1] / "fixtures" / "nothing.bin").read_bytes()
 
 
-def _pook_url():
+def _pook_url(URL):
     return pook.head(URL).reply(200).mock
 
 
-async def test_async_with_request():
-    mock = _pook_url()
+@pytest.fixture
+def URL(httpbin):
+    return f"{httpbin.url}/status/404"
+
+
+@pytest.mark.asyncio
+async def test_async_with_request(URL):
+    mock = _pook_url(URL)
     async with aiohttp.ClientSession() as session:
         async with session.head(URL) as req:
             assert req.status == 200
@@ -38,8 +43,9 @@ async def test_async_with_request():
     assert len(mock.matches) == 1
 
 
-async def test_await_request():
-    mock = _pook_url()
+@pytest.mark.asyncio
+async def test_await_request(URL):
+    mock = _pook_url(URL)
     async with aiohttp.ClientSession() as session:
         req = await session.head(URL)
         assert req.status == 200
@@ -47,7 +53,8 @@ async def test_await_request():
     assert len(mock.matches) == 1
 
 
-async def test_binary_body():
+@pytest.mark.asyncio
+async def test_binary_body(URL):
     pook.get(URL).reply(200).body(binary_file, binary=True)
     async with aiohttp.ClientSession() as session:
         req = await session.get(URL)
diff --git a/tests/unit/interceptors/base.py b/tests/unit/interceptors/base.py
new file mode 100644
index 0000000..ee5effb
--- /dev/null
+++ b/tests/unit/interceptors/base.py
@@ -0,0 +1,58 @@
+import asyncio
+from typing import Optional, Tuple
+
+import pytest
+
+import pook
+
+
+class StandardTests:
+    is_async: bool = False
+
+    async def amake_request(self, method: str, url: str) -> Tuple[int, Optional[str]]:
+        raise NotImplementedError(
+            "Sub-classes for async transports must implement `amake_request`"
+        )
+
+    def make_request(self, method: str, url: str) -> Tuple[int, Optional[str]]:
+        if self.is_async:
+            return self.loop.run_until_complete(self.amake_request(method, url))
+
+        raise NotImplementedError("Sub-classes must implement `make_request`")
+
+    @pytest.fixture(autouse=True, scope="class")
+    def _loop(self, request):
+        if self.is_async:
+            request.cls.loop = asyncio.new_event_loop()
+            yield
+            request.cls.loop.close()
+        else:
+            yield
+
+    @pytest.mark.pook
+    def test_activate_deactivate(self, httpbin):
+        url = f"{httpbin.url}/status/404"
+        pook.get(url).reply(200).body("hello from pook")
+
+        status, body = self.make_request("GET", url)
+
+        assert status == 200
+        assert body == "hello from pook"
+
+        pook.disable()
+
+        status, body = self.make_request("GET", url)
+
+        assert status == 404
+
+    @pytest.mark.pook(allow_pending_mocks=True)
+    def test_network_mode(self, httpbin):
+        upstream_url = f"{httpbin.url}/status/500"
+        mocked_url = f"{httpbin.url}/status/404"
+        pook.get(mocked_url).reply(200).body("hello from pook")
+        pook.enable_network()
+
+        # Avoid matching the mocks
+        status, body = self.make_request("POST", upstream_url)
+
+        assert status == 500
diff --git a/tests/unit/interceptors/httpx_test.py b/tests/unit/interceptors/httpx_test.py
index 968538d..c9acc45 100644
--- a/tests/unit/interceptors/httpx_test.py
+++ b/tests/unit/interceptors/httpx_test.py
@@ -4,14 +4,35 @@
 
 from itertools import zip_longest
 
-
-URL = "https://httpbin.org/status/404"
+from tests.unit.interceptors.base import StandardTests
 
 
 pytestmark = [pytest.mark.pook]
 
 
-def test_sync():
+class TestStandardAsyncHttpx(StandardTests):
+    is_async = True
+
+    async def amake_request(self, method, url):
+        async with httpx.AsyncClient() as client:
+            response = await client.request(method=method, url=url)
+            content = await response.aread()
+            return response.status_code, content.decode("utf-8")
+
+
+class TestStandardSyncHttpx(StandardTests):
+    def make_request(self, method, url):
+        response = httpx.request(method=method, url=url)
+        content = response.read()
+        return response.status_code, content.decode("utf-8")
+
+
+@pytest.fixture
+def URL(httpbin):
+    return f"{httpbin.url}/status/404"
+
+
+def test_sync(URL):
     pook.get(URL).times(1).reply(200).body("123")
 
     response = httpx.get(URL)
@@ -19,7 +40,7 @@ def test_sync():
     assert response.status_code == 200
 
 
-async def test_async():
+async def test_async(URL):
     pook.get(URL).times(1).reply(200).body(b"async_body", binary=True).mock
 
     async with httpx.AsyncClient() as client:
@@ -29,7 +50,7 @@ async def test_async():
     assert (await response.aread()) == b"async_body"
 
 
-def test_json():
+def test_json(URL):
     (
         pook.post(URL)
         .times(1)
@@ -44,7 +65,8 @@ def test_json():
     assert response.json() == {"title": "123abc title"}
 
 
-def _check_streaming_via(response_method):
+@pytest.mark.parametrize("response_method", ("iter_bytes", "iter_raw"))
+def test_streaming(URL, response_method):
     streamed_response = b"streamed response"
     pook.get(URL).times(1).reply(200).body(streamed_response).mock
 
@@ -55,15 +77,7 @@ def _check_streaming_via(response_method):
     assert bytes().join(read_bytes) == streamed_response
 
 
-def test_streaming_via_iter_bytes():
-    _check_streaming_via("iter_bytes")
-
-
-def test_streaming_via_iter_raw():
-    _check_streaming_via("iter_raw")
-
-
-def test_redirect_following():
+def test_redirect_following(URL):
     urls = [URL, f"{URL}/redirected", f"{URL}/redirected_again"]
     for req, dest in zip_longest(urls, urls[1:], fillvalue=None):
         if not dest:
diff --git a/tests/unit/interceptors/module_test.py b/tests/unit/interceptors/module_test.py
index af829a1..ddbc7e0 100644
--- a/tests/unit/interceptors/module_test.py
+++ b/tests/unit/interceptors/module_test.py
@@ -2,7 +2,11 @@
 
 
 class CustomInterceptor(interceptors.BaseInterceptor):
-    pass
+    def activate(self):
+        ...
+
+    def disable(self):
+        ...
 
 
 def test_add_custom_interceptor():
diff --git a/tests/unit/interceptors/urllib3_test.py b/tests/unit/interceptors/urllib3_test.py
index e5dd2a3..78ead1a 100644
--- a/tests/unit/interceptors/urllib3_test.py
+++ b/tests/unit/interceptors/urllib3_test.py
@@ -4,15 +4,26 @@
 
 from pathlib import Path
 
+from tests.unit.interceptors.base import StandardTests
+
 
 binary_file = (Path(__file__).parents[1] / "fixtures" / "nothing.bin").read_bytes()
 
 
-URL = "https://httpbin.org/foo"
+class TestStandardUrllib3(StandardTests):
+    def make_request(self, method, url):
+        http = urllib3.PoolManager()
+        response = http.request(method, url)
+        return response.status, response.read().decode("utf-8")
+
+
+@pytest.fixture
+def URL(httpbin):
+    return f"{httpbin.url}/foo"
 
 
 @pook.on
-def assert_chunked_response(input_data, expected):
+def assert_chunked_response(URL, input_data, expected):
     (pook.get(URL).reply(204).body(input_data, chunked=True))
 
     http = urllib3.PoolManager()
@@ -25,24 +36,24 @@ def assert_chunked_response(input_data, expected):
     assert chunks == expected
 
 
-def test_chunked_response_list():
-    assert_chunked_response(["a", "b", "c"], ["a", "b", "c"])
+def test_chunked_response_list(URL):
+    assert_chunked_response(URL, ["a", "b", "c"], ["a", "b", "c"])
 
 
-def test_chunked_response_str():
-    assert_chunked_response("text", ["text"])
+def test_chunked_response_str(URL):
+    assert_chunked_response(URL, "text", ["text"])
 
 
-def test_chunked_response_byte():
-    assert_chunked_response(b"byteman", ["byteman"])
+def test_chunked_response_byte(URL):
+    assert_chunked_response(URL, b"byteman", ["byteman"])
 
 
-def test_chunked_response_empty():
-    assert_chunked_response("", [])
+def test_chunked_response_empty(URL):
+    assert_chunked_response(URL, "", [])
 
 
-def test_chunked_response_contains_newline():
-    assert_chunked_response("newline\r\n", ["newline\r\n"])
+def test_chunked_response_contains_newline(URL):
+    assert_chunked_response(URL, "newline\r\n", ["newline\r\n"])
 
 
 def test_activate_disable():
@@ -56,7 +67,7 @@ def test_activate_disable():
 
 
 @pook.on
-def test_binary_body():
+def test_binary_body(URL):
     (pook.get(URL).reply(200).body(binary_file, binary=True))
 
     http = urllib3.PoolManager()
@@ -66,7 +77,7 @@ def test_binary_body():
 
 
 @pook.on
-def test_binary_body_chunked():
+def test_binary_body_chunked(URL):
     (pook.get(URL).reply(200).body(binary_file, binary=True, chunked=True))
 
     http = urllib3.PoolManager()
@@ -76,7 +87,7 @@ def test_binary_body_chunked():
 
 
 @pytest.mark.pook
-def test_post_with_headers():
+def test_post_with_headers(URL):
     mock = pook.post(URL).header("k", "v").reply(200).mock
     http = urllib3.PoolManager(headers={"k": "v"})
     resp = http.request("POST", URL)
diff --git a/tests/unit/interceptors/urllib_test.py b/tests/unit/interceptors/urllib_test.py
index f9534a1..901b4c3 100644
--- a/tests/unit/interceptors/urllib_test.py
+++ b/tests/unit/interceptors/urllib_test.py
@@ -1,8 +1,25 @@
 import pook
-from urllib.request import urlopen
+from urllib.error import HTTPError
+from urllib.request import urlopen, Request
 import pytest
 
 
+from tests.unit.interceptors.base import StandardTests
+
+
+class TestUrllib(StandardTests):
+    def make_request(self, method, url):
+        request = Request(
+            url=url,
+            method=method,
+        )
+        try:
+            response = urlopen(request)
+            return response.status, response.read()
+        except HTTPError as e:
+            return e.code, e.msg
+
+
 @pytest.mark.pook
 def test_urllib_ssl():
     pook.get("https://example.com").reply(200).body("Hello from pook")
diff --git a/tests/unit/matchers/query_test.py b/tests/unit/matchers/query_test.py
index c2856d4..61bb4b5 100644
--- a/tests/unit/matchers/query_test.py
+++ b/tests/unit/matchers/query_test.py
@@ -5,17 +5,22 @@
 from pook.exceptions import PookNoMatches
 
 
+@pytest.fixture
+def URL(httpbin):
+    return f"{httpbin.url}/status/404"
+
+
 @pytest.mark.pook(allow_pending_mocks=True)
-def test_param_exists_empty_disallowed():
-    pook.get("https://httpbin.org/404").param_exists("x").reply(200)
+def test_param_exists_empty_disallowed(URL):
+    pook.get(URL).param_exists("x").reply(200)
 
     with pytest.raises(PookNoMatches):
-        urlopen("https://httpbin.org/404?x")
+        urlopen(f"{URL}?x")
 
 
 @pytest.mark.pook
-def test_param_exists_empty_allowed():
-    pook.get("https://httpbin.org/404").param_exists("x", allow_empty=True).reply(200)
+def test_param_exists_empty_allowed(URL):
+    pook.get(URL).param_exists("x", allow_empty=True).reply(200)
 
-    res = urlopen("https://httpbin.org/404?x")
+    res = urlopen(f"{URL}?x")
     assert res.status == 200
diff --git a/tests/unit/mock_engine_test.py b/tests/unit/mock_engine_test.py
index 5b83476..a72b2ae 100644
--- a/tests/unit/mock_engine_test.py
+++ b/tests/unit/mock_engine_test.py
@@ -58,14 +58,13 @@ def test_mock_engine_status(engine):
     reason="Pook cannot disambiguate the two mocks. Ideally it would try to find the most specific mock that matches, but that's not possible yet."
 )
 @pytest.mark.pook(allow_pending_mocks=True)
-def test_mock_specificity():
-    pook.get("https://httpbin.org/404").header_present("authorization").reply(201)
-    pook.get("https://httpbin.org/404").headers({"Authorization": "Bearer pook"}).reply(
-        200
-    )
+def test_mock_specificity(httpbin):
+    url404 = f"{httpbin.url}/status/404"
+    pook.get(url404).header_present("authorization").reply(201)
+    pook.get(url404).headers({"Authorization": "Bearer pook"}).reply(200)
 
     res_with_headers = urlopen(
-        Request("https://httpbin.org/404", headers={"Authorization": "Bearer pook"})
+        Request(url404, headers={"Authorization": "Bearer pook"})
     )
 
     assert res_with_headers.status == 200