Return the correct headers type for mocked urllib responses (#154)

* Return correct header type for urllib response

Also add tests for headers for all interceptors

* Add standard tests implementation for requests

* Fix PR template typo

* Add provisional history entries

Author: sarayourfriend

.github/pull_request_template.md

@@ -5,5 +5,5 @@
 
 ## PR Checklist
 
-- [ ] I've added tests any code changes
+- [ ] I've added tests for any code changes
 - [ ] I've documented any new features

History.rst

@@ -1,6 +1,12 @@
 History
 =======
 
+vX.Y.Z / 20xx-xx-xx
+-------------------------
+
+  * Return the correct type of ``headers`` object for standard library urllib by @sarayourfriend in https://github.com/h2non/pook/pull/154.
+  * Support ``Sequence[tuple[str, str]]`` header input with aiohttp by @sarayourfriend in https://github.com/h2non/pook/pull/154.
+
 v2.1.1 / 2024-10-15
 -------------------------
 

src/pook/interceptors/aiohttp.py

@@ -2,6 +2,7 @@
 from http.client import responses as http_reasons
 from unittest import mock
 from urllib.parse import urlencode, urlunparse
+from collections.abc import Mapping
 
 from aiohttp.helpers import TimerNoop
 from aiohttp.streams import EmptyStreamReader
@@ -60,7 +61,24 @@ async def _on_request(
     ):
         # Create request contract based on incoming params
         req = Request(method)
-        req.headers = headers or {}
+
+        # aiohttp's interface allows various mappings, as well as an iterable of key/value tuples
+        # ``pook.request`` only allows a dict, so we need to map the iterable to the matchable interface
+        if headers:
+            if isinstance(headers, Mapping):
+                req.headers = headers
+            else:
+                req_headers = {}
+                # If it isn't a mapping, then its an Iterable[Tuple[Union[str, istr], str]]
+                for req_header, req_header_value in headers:
+                    normalised_header = req_header.lower()
+                    if normalised_header in req_headers:
+                        req_headers[normalised_header] += f", {req_header_value}"
+                    else:
+                        req_headers[normalised_header] = req_header_value
+
+                req.headers = req_headers
+
         req.body = data
 
         # Expose extra variadic arguments

src/pook/interceptors/http.py

@@ -1,5 +1,5 @@
 import socket
-from http.client import _CS_REQ_SENT  # type: ignore[attr-defined]
+from http.client import _CS_REQ_SENT, HTTPMessage  # type: ignore[attr-defined]
 from http.client import HTTPSConnection
 
 from http.client import (
@@ -69,18 +69,16 @@ def _on_request(self, _request, conn, method, url, body=None, headers=None, **kw
         # Shortcut to mock response
         res = mock._response
 
-        # Aggregate headers as list of tuples for interface compatibility
-        headers = []
-        for key in res._headers:
-            headers.append((key, res._headers[key]))
-
         mockres = HTTPResponse(SocketMock(), method=method, url=url)
         mockres.version = (1, 1)
         mockres.status = res._status
         # urllib requires `code` to be set, rather than `status`
         mockres.code = res._status
         mockres.reason = http_reasons.get(res._status)
-        mockres.headers = res._headers.to_dict()
+        mockres.headers = HTTPMessage()
+
+        for hkey, hval in res._headers.itermerged():
+            mockres.headers.add_header(hkey, hval)
 
         def getresponse():
             return mockres

tests/unit/interceptors/aiohttp_test.py

@@ -11,11 +11,13 @@
 class TestStandardAiohttp(StandardTests):
     is_async = True
 
-    async def amake_request(self, method, url, content=None):
+    async def amake_request(self, method, url, content=None, headers=None):
         async with aiohttp.ClientSession(loop=self.loop) as session:
-            req = await session.request(method=method, url=url, data=content)
-            response_content = await req.read()
-            return req.status, response_content
+            response = await session.request(
+                method=method, url=url, data=content, headers=headers
+            )
+            response_content = await response.read()
+            return response.status, response_content, response.headers
 
 
 def _pook_url(URL):

tests/unit/interceptors/base.py

@@ -1,6 +1,7 @@
 import asyncio
+from collections.abc import Sequence
 import json
-from typing import Optional, Tuple, Union
+from typing import Mapping, Optional, Tuple
 
 import pytest
 
@@ -12,18 +13,26 @@ class StandardTests:
     loop: asyncio.AbstractEventLoop
 
     async def amake_request(
-        self, method: str, url: str, content: Union[bytes, None] = None
-    ) -> Tuple[int, Optional[bytes]]:
+        self,
+        method: str,
+        url: str,
+        content: Optional[bytes] = None,
+        headers: Optional[Sequence[tuple[str, str]]] = None,
+    ) -> Tuple[int, Optional[bytes], Mapping[str, str]]:
         raise NotImplementedError(
             "Sub-classes for async transports must implement `amake_request`"
         )
 
     def make_request(
-        self, method: str, url: str, content: Union[bytes, None] = None
-    ) -> Tuple[int, Optional[bytes]]:
+        self,
+        method: str,
+        url: str,
+        content: Optional[bytes] = None,
+        headers: Optional[Sequence[tuple[str, str]]] = None,
+    ) -> Tuple[int, Optional[bytes], Mapping[str, str]]:
         if self.is_async:
             return self.loop.run_until_complete(
-                self.amake_request(method, url, content)
+                self.amake_request(method, url, content, headers)
             )
 
         raise NotImplementedError("Sub-classes must implement `make_request`")
@@ -37,68 +46,127 @@ def _loop(self, request):
         else:
             yield
 
+    @pytest.fixture
+    def url_404(self, httpbin):
+        """404 httpbin URL.
+
+        Useful in tests if pook is configured to reply 200, and the status is checked.
+        If pook does not match the request (and if that was the intended behaviour)
+        then the 404 status code makes that obvious!"""
+        return f"{httpbin.url}/status/404"
+
+    @pytest.fixture
+    def url_500(self, httpbin):
+        return f"{httpbin.url}/status/500"
+
     @pytest.mark.pook
-    def test_activate_deactivate(self, httpbin):
-        url = f"{httpbin.url}/status/404"
-        pook.get(url).reply(200).body("hello from pook")
+    def test_activate_deactivate(self, url_404):
+        """Deactivating pook allows requests to go through."""
+        pook.get(url_404).reply(200).body("hello from pook")
 
-        status, body = self.make_request("GET", url)
+        status, body, *_ = self.make_request("GET", url_404)
 
         assert status == 200
         assert body == b"hello from pook"
 
         pook.disable()
 
-        status, body = self.make_request("GET", url)
+        status, body, *_ = self.make_request("GET", url_404)
 
         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")
+    def test_network_mode(self, url_404, url_500):
+        """Enabling network mode allows requests to pass through even if no mock is matched."""
+        pook.get(url_404).reply(200).body("hello from pook")
         pook.enable_network()
 
         # Avoid matching the mocks
-        status, body = self.make_request("POST", upstream_url)
+        status, *_ = self.make_request("POST", url_500)
 
         assert status == 500
 
     @pytest.mark.pook
-    def test_json_request(self, httpbin):
-        url = f"{httpbin.url}/status/404"
+    def test_json_request(self, url_404):
+        """JSON request bodies are correctly matched."""
         json_request = {"hello": "json-request"}
-        pook.get(url).json(json_request).reply(200).body("hello from pook")
+        pook.get(url_404).json(json_request).reply(200).body("hello from pook")
 
-        status, body = self.make_request("GET", url, json.dumps(json_request).encode())
+        status, body, *_ = self.make_request(
+            "GET", url_404, json.dumps(json_request).encode()
+        )
 
         assert status == 200
         assert body == b"hello from pook"
 
     @pytest.mark.pook
-    def test_json_response(self, httpbin):
-        url = f"{httpbin.url}/status/404"
+    def test_json_response(self, url_404):
+        """JSON responses are correctly mocked."""
         json_response = {"hello": "json-request"}
-        pook.get(url).reply(200).json(json_response)
+        pook.get(url_404).reply(200).json(json_response)
 
-        status, body = self.make_request("GET", url)
+        status, body, *_ = self.make_request("GET", url_404)
 
         assert status == 200
         assert body
         assert json.loads(body) == json_response
 
     @pytest.mark.pook
-    def test_json_request_and_response(self, httpbin):
-        url = f"{httpbin.url}/status/404"
+    def test_json_request_and_response(self, url_404):
+        """JSON requests and responses do not interfere with each other."""
         json_request = {"id": "123abc"}
         json_response = {"title": "123abc title"}
-        pook.get(url).json(json_request).reply(200).json(json_response)
+        pook.get(url_404).json(json_request).reply(200).json(json_response)
 
-        status, body = self.make_request(
-            "GET", url, content=json.dumps(json_request).encode()
+        status, body, *_ = self.make_request(
+            "GET", url_404, content=json.dumps(json_request).encode()
         )
 
         assert status == 200
         assert body
         assert json.loads(body) == json_response
+
+    @pytest.mark.pook
+    def test_header_sent(self, url_404):
+        """Sent headers can be matched."""
+        headers = [("x-hello", "from pook")]
+        pook.get(url_404).header("x-hello", "from pook").reply(200).body(
+            "hello from pook"
+        )
+
+        status, body, _ = self.make_request("GET", url_404, headers=headers)
+
+        assert status == 200
+        assert body == b"hello from pook"
+
+    @pytest.mark.pook
+    def test_mocked_resposne_headers(self, url_404):
+        """Mocked response headers are appropriately returned."""
+        pook.get(url_404).reply(200).header("x-hello", "from pook")
+
+        status, _, headers = self.make_request("GET", url_404)
+
+        assert status == 200
+        assert headers["x-hello"] == "from pook"
+
+    @pytest.mark.pook
+    def test_mutli_value_headers(self, url_404):
+        """Multi-value headers can be matched."""
+        match_headers = [("x-hello", "from pook"), ("x-hello", "another time")]
+        pook.get(url_404).header("x-hello", "from pook, another time").reply(200)
+
+        status, *_ = self.make_request("GET", url_404, headers=match_headers)
+
+        assert status == 200
+
+    @pytest.mark.pook
+    def test_mutli_value_response_headers(self, url_404):
+        """Multi-value response headers can be mocked."""
+        pook.get(url_404).reply(200).header("x-hello", "from pook").header(
+            "x-hello", "another time"
+        )
+
+        status, _, headers = self.make_request("GET", url_404)
+
+        assert status == 200
+        assert headers["x-hello"] == "from pook, another time"

tests/unit/interceptors/httpx_test.py

@@ -12,18 +12,22 @@
 class TestStandardAsyncHttpx(StandardTests):
     is_async = True
 
-    async def amake_request(self, method, url, content=None):
+    async def amake_request(self, method, url, content=None, headers=None):
         async with httpx.AsyncClient() as client:
-            response = await client.request(method=method, url=url, content=content)
+            response = await client.request(
+                method=method, url=url, content=content, headers=headers
+            )
             content = await response.aread()
-            return response.status_code, content
+            return response.status_code, content, response.headers
 
 
 class TestStandardSyncHttpx(StandardTests):
-    def make_request(self, method, url, content=None):
-        response = httpx.request(method=method, url=url, content=content)
+    def make_request(self, method, url, content=None, headers=None):
+        response = httpx.request(
+            method=method, url=url, content=content, headers=headers
+        )
         content = response.read()
-        return response.status_code, content
+        return response.status_code, content, response.headers
 
 
 @pytest.fixture

tests/unit/interceptors/urllib3_test.py

@@ -1,16 +1,39 @@
 import pytest
 import urllib3
+import requests
 
 import pook
 from tests.unit.fixtures import BINARY_FILE
 from tests.unit.interceptors.base import StandardTests
 
 
 class TestStandardUrllib3(StandardTests):
-    def make_request(self, method, url, content=None):
+    def make_request(self, method, url, content=None, headers=None):
+        req_headers = {}
+        if headers:
+            for header, value in headers:
+                if header in req_headers:
+                    req_headers[header] += f", {value}"
+                else:
+                    req_headers[header] = value
+
         http = urllib3.PoolManager()
-        response = http.request(method, url, content)
-        return response.status, response.read()
+        response = http.request(method, url, content, headers=req_headers)
+        return response.status, response.read(), response.headers
+
+
+class TestStandardRequests(StandardTests):
+    def make_request(self, method, url, content=None, headers=None):
+        req_headers = {}
+        if headers:
+            for header, value in headers:
+                if header in req_headers:
+                    req_headers[header] += f", {value}"
+                else:
+                    req_headers[header] = value
+
+        response = requests.request(method, url, data=content, headers=req_headers)
+        return response.status_code, response.content, response.headers
 
 
 @pytest.fixture