From cc213a3ac4bafa6576069b90458ee9332bab0881 Mon Sep 17 00:00:00 2001 From: sarayourfriend Date: Mon, 17 Jun 2024 16:24:38 +1000 Subject: [PATCH] Revert "Deprecate non-binary bodies (#130)" This reverts commit 1e930cb02f2fbbe8cde15ab90a8dc54dda8eb0e3. --- History.rst | 7 -- src/pook/api.py | 3 +- src/pook/interceptors/http.py | 6 +- src/pook/interceptors/urllib3.py | 4 +- src/pook/mock.py | 2 +- src/pook/response.py | 96 +------------------------ tests/conftest.py | 39 ---------- tests/unit/interceptors/aiohttp_test.py | 12 +--- tests/unit/interceptors/base.py | 28 ++------ tests/unit/interceptors/httpx_test.py | 4 +- tests/unit/interceptors/urllib3_test.py | 47 +++--------- tests/unit/interceptors/urllib_test.py | 6 +- tests/unit/mock_test.py | 4 +- tests/unit/warnings_test.py | 78 -------------------- 14 files changed, 26 insertions(+), 310 deletions(-) delete mode 100644 tests/conftest.py delete mode 100644 tests/unit/warnings_test.py diff --git a/History.rst b/History.rst index c1f7505..45a59da 100644 --- a/History.rst +++ b/History.rst @@ -1,13 +1,6 @@ History ======= -v1.5.0 / 2024-06-17 -------------------- - - * Provide fix for inaccurate non-binary response bodies from urllib and deprecate the `binary` body parameter entirely - * Refer to https://github.com/h2non/pook/issues/128 for further details - * Fix body matcher binary argument not passing through to `BodyMatcher` - v1.4.3 / 2024-02-23 ------------------- diff --git a/src/pook/api.py b/src/pook/api.py index efac738..4073f7e 100644 --- a/src/pook/api.py +++ b/src/pook/api.py @@ -8,7 +8,7 @@ from .mock import Mock from .mock_engine import MockEngine from .request import Request -from .response import Response, apply_binary_body_fix +from .response import Response from asyncio import iscoroutinefunction from .activate_async import activate_async @@ -16,7 +16,6 @@ # Public API symbols to export __all__ = ( "activate", - "apply_binary_body_fix", "on", "disable", "off", diff --git a/src/pook/interceptors/http.py b/src/pook/interceptors/http.py index a42b339..6645705 100644 --- a/src/pook/interceptors/http.py +++ b/src/pook/interceptors/http.py @@ -1,6 +1,5 @@ import socket from ..request import Request -from ..response import _BinaryDefault from .base import BaseInterceptor from unittest import mock @@ -39,9 +38,6 @@ def close(self, *args, **kw): pass -_body_fallback = b"" if _BinaryDefault.fixed else "" - - class HTTPClientInterceptor(BaseInterceptor): """ urllib / http.client HTTP traffic interceptor. @@ -96,7 +92,7 @@ def getresponse(): # Path reader def read(): - return res._body or _body_fallback + return res._body or "" mockres.read = read diff --git a/src/pook/interceptors/urllib3.py b/src/pook/interceptors/urllib3.py index 23980c6..a9ca23d 100644 --- a/src/pook/interceptors/urllib3.py +++ b/src/pook/interceptors/urllib3.py @@ -152,10 +152,8 @@ def _on_request( if is_chunked_response(headers): body_chunks = body if isinstance(body, list) else [body] - body_chunks = [ - chunk.encode() if callable(getattr(chunk, "encode", None)) else chunk - for chunk in body_chunks + chunk if res._binary else chunk.encode() for chunk in body_chunks ] body = ClientHTTPResponse(MockSock) diff --git a/src/pook/mock.py b/src/pook/mock.py index 09a567a..ae606d0 100644 --- a/src/pook/mock.py +++ b/src/pook/mock.py @@ -410,7 +410,7 @@ def body(self, body, binary=False): self: current Mock instance. """ self._request.body = body - self.add_matcher(matcher("BodyMatcher", body, binary=binary)) + self.add_matcher(matcher("BodyMatcher", body, binary=False)) return self def json(self, json): diff --git a/src/pook/response.py b/src/pook/response.py index 9756f6a..9effd56 100644 --- a/src/pook/response.py +++ b/src/pook/response.py @@ -1,50 +1,9 @@ import json -import warnings -from textwrap import dedent - from .headers import HTTPHeaderDict from .helpers import trigger_methods from .constants import TYPES -class _BinaryDefault: - fixed = False - - start_warning = " ".join( - dedent( - """ - Non-binary pook response bodies are deprecated. - Support for them will be removed in the next major version of pook, - and responses will always be bytes, matching the behaviour of all - client libraries that pook supports. - """ - ) - .strip() - .split("\n") - ) - - end_warning = " ".join( - dedent( - """ - Refer to https://github.com/h2non/pook/issues/128 for further details. - In most circumstances, your existing code will continue to work without changes. - If you do not use pook to mock urllib (e.g., `urlopen` or a library that wraps it), - then you are not affected and can safely ignore this warning. - """ - ) - .strip() - .split("\n") - ) - - @classmethod - def make_warning(cls, text): - return f"{cls.start_warning} {text} {cls.end_warning}" - - -def apply_binary_body_fix(): - _BinaryDefault.fixed = True - - class Response(object): """ Response is used to declare and compose an HTTP mock responses fields. @@ -199,7 +158,7 @@ def content(self, name): self._headers["Content-Type"] = TYPES.get(name, name) return self - def body(self, body, binary=_BinaryDefault, chunked=False): + def body(self, body, binary=False, chunked=False): """ Defines response body data. @@ -211,59 +170,6 @@ def body(self, body, binary=_BinaryDefault, chunked=False): Returns: self: ``pook.Response`` current instance. """ - apply_fix = False - if binary is _BinaryDefault: - if _BinaryDefault.fixed: - # Fixed and not explicitly passing binary, perfect! Ready for the future! - apply_fix = True - else: - warnings.warn( - _BinaryDefault.make_warning( - "Call `pook.apply_binary_body_fix()` at least once to resolve this notice." - ), - DeprecationWarning, - stacklevel=2, - ) - binary = False - - else: # explicitly True or False - if _BinaryDefault.fixed: - # Fixed, but explicitly passing `binary` - warnings.warn( - _BinaryDefault.make_warning( - "The fix is already applied, but `binary` was explicitly passed to `.body()`. " - "Remove `binary` from this body call, and update any application code that treated " - "the response as anything other than bytes." - ), - DeprecationWarning, - stacklevel=2, - ) - if binary: - apply_fix = True - else: - # Not fixed and explicitly passing `binary` - warnings.warn( - _BinaryDefault.make_warning( - "To resolve this notice, call `pook.apply_binary_body_fix()` in your " - "conftest (or equivalent). " - "Then, remove `binary` from this body call, and update any application code that treated " - "the response as anything other than bytes." - ), - DeprecationWarning, - stacklevel=2, - ) - - if apply_fix: - binary = True - - if isinstance(body, list): - for i, chunk in enumerate(body): - if hasattr(chunk, "encode"): - body[i] = chunk.encode() - - if hasattr(body, "encode"): - body = body.encode() - if isinstance(body, bytes) and not binary: body = body.decode("utf-8") diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index c36e601..0000000 --- a/tests/conftest.py +++ /dev/null @@ -1,39 +0,0 @@ -import pytest -import warnings - -import pook -from pook.response import _BinaryDefault - - -pook.apply_binary_body_fix() - - -def pytest_configure(config: pytest.Config): - config.addinivalue_line( - "markers", - "undo_suppress_own_warnings: undo pook-warning suppression", - ) - - -@pytest.fixture(autouse=True) -def _suppress_own_deprecation_warnings(request: pytest.FixtureRequest): - undo = request.node.get_closest_marker("undo_suppress_own_warnings") - - if undo: - # Only relevant when testing the warnings themselves - yield - return - - with warnings.catch_warnings(record=True) as recorded_warnings: - yield - - for warning in recorded_warnings: - if "Non-binary pook response bodies are deprecated" not in str(warning.message): - warnings.showwarning(warning) - - -@pytest.fixture(scope="function") -def without_binary_body_fix(): - _BinaryDefault.fixed = False - yield - pook.apply_binary_body_fix() diff --git a/tests/unit/interceptors/aiohttp_test.py b/tests/unit/interceptors/aiohttp_test.py index bc7de9f..a29009a 100644 --- a/tests/unit/interceptors/aiohttp_test.py +++ b/tests/unit/interceptors/aiohttp_test.py @@ -18,7 +18,7 @@ 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 + return req.status, content.decode("utf-8") binary_file = (Path(__file__).parents[1] / "fixtures" / "nothing.bin").read_bytes() @@ -53,17 +53,9 @@ async def test_await_request(URL): assert len(mock.matches) == 1 -@pytest.mark.asyncio -async def test_binary_body_deprecated(URL, without_binary_body_fix): - pook.get(URL).reply(200).body(binary_file, binary=True) - async with aiohttp.ClientSession() as session: - req = await session.get(URL) - assert await req.read() == binary_file - - @pytest.mark.asyncio async def test_binary_body(URL): - pook.get(URL).reply(200).body(binary_file) + pook.get(URL).reply(200).body(binary_file, binary=True) async with aiohttp.ClientSession() as session: req = await session.get(URL) assert await req.read() == binary_file diff --git a/tests/unit/interceptors/base.py b/tests/unit/interceptors/base.py index 6659ce4..ee5effb 100644 --- a/tests/unit/interceptors/base.py +++ b/tests/unit/interceptors/base.py @@ -1,5 +1,5 @@ import asyncio -from typing import Optional, Tuple, Union +from typing import Optional, Tuple import pytest @@ -8,18 +8,13 @@ class StandardTests: is_async: bool = False - requires_binary_body_fix: bool = False - async def amake_request( - self, method: str, url: str - ) -> Tuple[int, Optional[Union[str, bytes]]]: + 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[Union[str, bytes]]]: + 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)) @@ -42,7 +37,7 @@ def test_activate_deactivate(self, httpbin): status, body = self.make_request("GET", url) assert status == 200 - assert body == b"hello from pook" + assert body == "hello from pook" pook.disable() @@ -61,18 +56,3 @@ def test_network_mode(self, httpbin): status, body = self.make_request("POST", upstream_url) assert status == 500 - - @pytest.mark.pook - def test_binary_body_deprecated(self, httpbin, without_binary_body_fix): - 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 - - if self.requires_binary_body_fix: - assert body == "hello from pook" - else: - assert body == b"hello from pook" diff --git a/tests/unit/interceptors/httpx_test.py b/tests/unit/interceptors/httpx_test.py index 361aa2a..c9acc45 100644 --- a/tests/unit/interceptors/httpx_test.py +++ b/tests/unit/interceptors/httpx_test.py @@ -17,14 +17,14 @@ 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 + 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 + return response.status_code, content.decode("utf-8") @pytest.fixture diff --git a/tests/unit/interceptors/urllib3_test.py b/tests/unit/interceptors/urllib3_test.py index f39e3d3..78ead1a 100644 --- a/tests/unit/interceptors/urllib3_test.py +++ b/tests/unit/interceptors/urllib3_test.py @@ -1,7 +1,6 @@ import urllib3 import pook import pytest -import requests from pathlib import Path @@ -15,16 +14,7 @@ class TestStandardUrllib3(StandardTests): def make_request(self, method, url): http = urllib3.PoolManager() response = http.request(method, url) - return response.status, response.read() - - -class TestStandardRequests(StandardTests): - def make_request(self, method, url): - res = requests.request( - method=method, - url=url, - ) - return res.status_code, res.content + return response.status, response.read().decode("utf-8") @pytest.fixture @@ -37,24 +27,25 @@ def assert_chunked_response(URL, input_data, expected): (pook.get(URL).reply(204).body(input_data, chunked=True)) http = urllib3.PoolManager() - r = http.request("GET", URL, decode_content=False) + r = http.request("GET", URL) assert r.status == 204 chunks = list(r.read_chunked()) + chunks = [c.decode() if isinstance(c, bytes) else c for c in chunks] assert chunks == expected def test_chunked_response_list(URL): - assert_chunked_response(URL, ["a", "b", "c"], [b"a", b"b", b"c"]) + assert_chunked_response(URL, ["a", "b", "c"], ["a", "b", "c"]) def test_chunked_response_str(URL): - assert_chunked_response(URL, "text", [b"text"]) + assert_chunked_response(URL, "text", ["text"]) def test_chunked_response_byte(URL): - assert_chunked_response(URL, b"byteman", [b"byteman"]) + assert_chunked_response(URL, b"byteman", ["byteman"]) def test_chunked_response_empty(URL): @@ -62,7 +53,7 @@ def test_chunked_response_empty(URL): def test_chunked_response_contains_newline(URL): - assert_chunked_response(URL, "newline\r\n", [b"newline\r\n"]) + assert_chunked_response(URL, "newline\r\n", ["newline\r\n"]) def test_activate_disable(): @@ -75,19 +66,9 @@ def test_activate_disable(): assert urllib3.connectionpool.HTTPConnectionPool.urlopen == original -@pook.on -def test_binary_body_deprecated(URL, without_binary_body_fix): - (pook.get(URL).reply(200).body(binary_file, binary=True)) - - http = urllib3.PoolManager() - r = http.request("GET", URL) - - assert r.read() == binary_file - - @pook.on def test_binary_body(URL): - (pook.get(URL).reply(200).body(binary_file)) + (pook.get(URL).reply(200).body(binary_file, binary=True)) http = urllib3.PoolManager() r = http.request("GET", URL) @@ -95,19 +76,9 @@ def test_binary_body(URL): assert r.read() == binary_file -@pook.on -def test_binary_body_chunked_deprecated(URL, without_binary_body_fix): - (pook.get(URL).reply(200).body(binary_file, binary=True, chunked=True)) - - http = urllib3.PoolManager() - r = http.request("GET", URL) - - assert list(r.read_chunked()) == [binary_file] - - @pook.on def test_binary_body_chunked(URL): - (pook.get(URL).reply(200).body(binary_file, chunked=True)) + (pook.get(URL).reply(200).body(binary_file, binary=True, chunked=True)) http = urllib3.PoolManager() r = http.request("GET", URL) diff --git a/tests/unit/interceptors/urllib_test.py b/tests/unit/interceptors/urllib_test.py index 0fd8ec9..901b4c3 100644 --- a/tests/unit/interceptors/urllib_test.py +++ b/tests/unit/interceptors/urllib_test.py @@ -8,8 +8,6 @@ class TestUrllib(StandardTests): - requires_binary_body_fix = True - def make_request(self, method, url): request = Request( url=url, @@ -27,7 +25,7 @@ def test_urllib_ssl(): pook.get("https://example.com").reply(200).body("Hello from pook") res = urlopen("https://example.com") - assert res.read() == b"Hello from pook" + assert res.read() == "Hello from pook" @pytest.mark.pook @@ -35,4 +33,4 @@ def test_urllib_clear(): pook.get("http://example.com").reply(200).body("Hello from pook") res = urlopen("http://example.com") - assert res.read() == b"Hello from pook" + assert res.read() == "Hello from pook" diff --git a/tests/unit/mock_test.py b/tests/unit/mock_test.py index 333ff3e..856a5e7 100644 --- a/tests/unit/mock_test.py +++ b/tests/unit/mock_test.py @@ -148,10 +148,10 @@ def test_times_integrated(httpbin): pook.get(url).times(2).reply(200).body("hello from pook") res = urlopen(url) - assert res.read() == b"hello from pook" + assert res.read() == "hello from pook" res = urlopen(url) - assert res.read() == b"hello from pook" + assert res.read() == "hello from pook" with pytest.raises(PookNoMatches, match="Mock matches request but is expired."): urlopen(url) diff --git a/tests/unit/warnings_test.py b/tests/unit/warnings_test.py deleted file mode 100644 index 2a8a6f6..0000000 --- a/tests/unit/warnings_test.py +++ /dev/null @@ -1,78 +0,0 @@ -import warnings -from urllib.request import urlopen - -import pytest - -import pook - - -pytestmark = [ - pytest.mark.undo_suppress_own_warnings, - pytest.mark.pook, -] - - -def test_deprecation_warning_fixed_no_binary_arg(httpbin): - with warnings.catch_warnings(record=True) as recorded_warnings: - # Best case scenario - url = f"{httpbin.url}/status/404" - pook.get(url).param("was", "not_bytes").reply(200).body("hello from pook") - pook.get(url).param("was", "bytes").reply(200).body( - b"hello with bytes argument" - ) - - res = urlopen(f"{url}?was=bytes") - assert res.read() == b"hello with bytes argument" - - res = urlopen(f"{url}?was=not_bytes") - assert res.read() == b"hello from pook" - - assert recorded_warnings == [] - - -def test_deprecation_warning_fixed_explicit_argument(httpbin): - with warnings.catch_warnings(record=True) as recorded_warnings: - # Best case scenario - url = f"{httpbin.url}/status/404" - pook.get(url).reply(200).body("hello from pook", binary=True) - - res = urlopen(url) - assert res.read() == b"hello from pook" - - assert len(recorded_warnings) == 1 - assert ( - "The fix is already applied, but `binary` was explicitly passed to `.body()`." - in str(recorded_warnings[0].message) - ) - - -def test_deprecation_warning_unapplied_fix_no_arg(httpbin, without_binary_body_fix): - with warnings.catch_warnings(record=True) as recorded_warnings: - # Best case scenario - url = f"{httpbin.url}/status/404" - pook.get(url).reply(200).body("hello from pook") - - res = urlopen(url) - assert res.read() == "hello from pook" - - assert len(recorded_warnings) == 1 - message = str(recorded_warnings[0].message) - assert "Support for them will be removed in the next major version" in message - assert "Call `pook.apply_binary_body_fix()` at least once" in message - - -def test_deprecation_warning_unapplied_fix_explicit_argument( - httpbin, without_binary_body_fix -): - with warnings.catch_warnings(record=True) as recorded_warnings: - # Best case scenario - url = f"{httpbin.url}/status/404" - pook.get(url).reply(200).body(b"hello from pook", binary=True) - - res = urlopen(url) - assert res.read() == b"hello from pook" - - assert len(recorded_warnings) == 1 - message = str(recorded_warnings[0].message) - assert "Support for them will be removed in the next major version" in message - assert "Then, remove `binary` from this body call" in message