Repository: getsentry/responses
Pull request #711 (2024-04-26 13:22:54)

fix(matchers): Don't sort failed matches when printing error message
Hi!

This PR removes the `_create_key_val_str` and with it the sorting when displaying matchers errors

Let me know any change required!

Thank you 

Fixes GH-704

beliaev-maksim: @markstory I think we introduced `_create_key_val_str` for the compatibility between python2 and python3

should we just drop it completely and just present JSON?
beliaev-maksim: @mgaligniana please add changelog file entry
mgaligniana: @beliaev-maksim thank you for your fast response! On it! :factory_worker: 
mgaligniana: @beliaev-maksim Done! Let me know if message is OK (I'm not English native speaker). Feel free to suggest changes! Thank you!
markstory: > I think we introduced _create_key_val_str for the compatibility between python2 and python3 should we just drop it completely and just present JSON?

I think you're right on the origin of that method. We could likely drop it now that we're not supporting python 2 anymore.


mgaligniana: @markstory Do you want me to tackle in this pull request? Or in another ticket?
beliaev-maksim: @mgaligniana yes please
markstory: > Do you want me to tackle in this pull request? Or in another ticket?

Either works for me. Using this pull request would be simpler for us though.
mgaligniana: Done!

I had to update all test cases because now it shows inputs are strings in error messages.

For example in `query_param_matcher`
Before: `...doesn't match {array: [B, A, C]}` After: `...doesn't match: {'array': ['B', 'A', 'C']}`
mgaligniana: > I would not talk about internal method. Instead, please briefly describe what changed for the end user

Makes sense!

Ready, thanks for your help and patience!
beliaev-maksim: looks good, thanks
markstory: There are a few failing tests that will need to be addressed before this could be merged.
mgaligniana: > There are a few failing tests that will need to be addressed before this could be merged.

Will star working on that! I'm going to use `tox` to can reproduce them! 👷 🚧 
mgaligniana: Now it is! I missed a f-string and probably forgot run the tests before push 🤷, sorry!

Patch:
diff --git a/CHANGES b/CHANGES
index a379d63..db4eb8f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,13 @@
+0.25.2
+------
+
+* Fixed error messages when matches fail: inputs are not sorted or reformatted. See #704
+
 0.25.1
 ------
 
 * Fixed tests failures during RPM package builds. See #706
+* Fix mocked HEAD responses that have `Content-Length` set. See #712
 
 0.25.0
 ------
diff --git a/responses/__init__.py b/responses/__init__.py
index d990091..39db1e7 100644
--- a/responses/__init__.py
+++ b/responses/__init__.py
@@ -532,6 +532,7 @@ def _form_response(
     body: Union[BufferedReader, BytesIO],
     headers: Optional[Mapping[str, str]],
     status: int,
+    request_method: Optional[str],
 ) -> HTTPResponse:
     """
     Function to generate `urllib3.response.HTTPResponse` object.
@@ -566,6 +567,7 @@ def _form_response(
         headers=headers,
         original_response=orig_response,  # type: ignore[arg-type]  # See comment above
         preload_content=False,
+        request_method=request_method,
     )
 
 
@@ -632,7 +634,7 @@ class Response(BaseResponse):
             content_length = len(body.getvalue())
             headers["Content-Length"] = str(content_length)
 
-        return _form_response(body, headers, status)
+        return _form_response(body, headers, status, request.method)
 
     def __repr__(self) -> str:
         return (
@@ -695,7 +697,7 @@ class CallbackResponse(BaseResponse):
         body = _handle_body(body)
         headers.extend(r_headers)
 
-        return _form_response(body, headers, status)
+        return _form_response(body, headers, status, request.method)
 
 
 class PassthroughResponse(BaseResponse):
diff --git a/responses/matchers.py b/responses/matchers.py
index 20af1be..c9f7a63 100644
--- a/responses/matchers.py
+++ b/responses/matchers.py
@@ -18,45 +18,6 @@ from requests import PreparedRequest
 from urllib3.util.url import parse_url
 
 
-def _create_key_val_str(input_dict: Union[Mapping[Any, Any], Any]) -> str:
-    """
-    Returns string of format {'key': val, 'key2': val2}
-    Function is called recursively for nested dictionaries
-
-    :param input_dict: dictionary to transform
-    :return: (str) reformatted string
-    """
-
-    def list_to_str(input_list: List[str]) -> str:
-        """
-        Convert all list items to string.
-        Function is called recursively for nested lists
-        """
-        converted_list = []
-        for item in sorted(input_list, key=lambda x: str(x)):
-            if isinstance(item, dict):
-                item = _create_key_val_str(item)
-            elif isinstance(item, list):
-                item = list_to_str(item)
-
-            converted_list.append(str(item))
-        list_str = ", ".join(converted_list)
-        return "[" + list_str + "]"
-
-    items_list = []
-    for key in sorted(input_dict.keys(), key=lambda x: str(x)):
-        val = input_dict[key]
-        if isinstance(val, dict):
-            val = _create_key_val_str(val)
-        elif isinstance(val, list):
-            val = list_to_str(input_list=val)
-
-        items_list.append(f"{key}: {val}")
-
-    key_val_str = "{{{}}}".format(", ".join(items_list))
-    return key_val_str
-
-
 def _filter_dict_recursively(
     dict1: Mapping[Any, Any], dict2: Mapping[Any, Any]
 ) -> Mapping[Any, Any]:
@@ -91,8 +52,8 @@ def urlencoded_params_matcher(
         params_dict = params or {}
         valid = params is None if request_body is None else params_dict == qsl_body
         if not valid:
-            reason = "request.body doesn't match: {} doesn't match {}".format(
-                _create_key_val_str(qsl_body), _create_key_val_str(params_dict)
+            reason = (
+                f"request.body doesn't match: {qsl_body} doesn't match {params_dict}"
             )
 
         return valid, reason
@@ -146,13 +107,7 @@ def json_params_matcher(
             valid = params is None if request_body is None else json_params == json_body
 
             if not valid:
-                if isinstance(json_body, dict) and isinstance(json_params, dict):
-                    reason = "request.body doesn't match: {} doesn't match {}".format(
-                        _create_key_val_str(json_body), _create_key_val_str(json_params)
-                    )
-                else:
-                    reason = f"request.body doesn't match: {json_body} doesn't match {json_params}"
-
+                reason = f"request.body doesn't match: {json_body} doesn't match {json_params}"
                 if not strict_match:
                     reason += (
                         "\nNote: You use non-strict parameters check, "
@@ -234,10 +189,7 @@ def query_param_matcher(
         valid = sorted(params_dict.items()) == sorted(request_params_dict.items())
 
         if not valid:
-            reason = "Parameters do not match. {} doesn't match {}".format(
-                _create_key_val_str(request_params_dict),
-                _create_key_val_str(params_dict),
-            )
+            reason = f"Parameters do not match. {request_params_dict} doesn't match {params_dict}"
             if not strict_match:
                 reason += (
                     "\nYou can use `strict_match=True` to do a strict parameters check."
@@ -267,9 +219,9 @@ def query_string_matcher(query: Optional[str]) -> Callable[..., Any]:
         valid = not query if request_query is None else request_qsl == matcher_qsl
 
         if not valid:
-            reason = "Query string doesn't match. {} doesn't match {}".format(
-                _create_key_val_str(dict(request_qsl)),
-                _create_key_val_str(dict(matcher_qsl)),
+            reason = (
+                "Query string doesn't match. "
+                f"{dict(request_qsl)} doesn't match {dict(matcher_qsl)}"
             )
 
         return valid, reason
@@ -299,8 +251,8 @@ def request_kwargs_matcher(kwargs: Optional[Mapping[str, Any]]) -> Callable[...,
         )
 
         if not valid:
-            reason = "Arguments don't match: {} doesn't match {}".format(
-                _create_key_val_str(request_kwargs), _create_key_val_str(kwargs_dict)
+            reason = (
+                f"Arguments don't match: {request_kwargs} doesn't match {kwargs_dict}"
             )
 
         return valid, reason
@@ -436,8 +388,9 @@ def header_matcher(
         valid = _compare_with_regex(request_headers)
 
         if not valid:
-            return False, "Headers do not match: {} doesn't match {}".format(
-                _create_key_val_str(request_headers), _create_key_val_str(headers)
+            return (
+                False,
+                f"Headers do not match: {request_headers} doesn't match {headers}",
             )
 
         return valid, ""


Test patch:
diff --git a/responses/tests/test_matchers.py b/responses/tests/test_matchers.py
index 796b93e..67d8e83 100644
--- a/responses/tests/test_matchers.py
+++ b/responses/tests/test_matchers.py
@@ -135,13 +135,27 @@ def test_json_params_matcher_not_strict_diff_values():
             )
         assert (
             "- POST http://example.com/ request.body doesn't match: "
-            "{page: {type: json}} doesn't match {page: {diff: value, type: json}}"
+            "{'page': {'type': 'json'}} doesn't match {'page': {'type': 'json', 'diff': 'value'}}"
         ) in str(exc.value)
 
     run()
     assert_reset()
 
 
+def test_failed_matchers_dont_modify_inputs_order_in_error_message():
+    json_a = {"array": ["C", "B", "A"]}
+    json_b = '{"array" : ["B", "A", "C"]}'
+    mock_request = Mock(body=json_b)
+    result = matchers.json_params_matcher(json_a)(mock_request)
+    assert result == (
+        False,
+        (
+            "request.body doesn't match: {'array': ['B', 'A', 'C']} "
+            "doesn't match {'array': ['C', 'B', 'A']}"
+        ),
+    )
+
+
 def test_json_params_matcher_json_list():
     json_a = [{"a": "b"}]
     json_b = '[{"a": "b", "c": "d"}]'
@@ -250,7 +264,7 @@ def test_query_param_matcher_loose_fail():
 
         assert (
             "- GET https://example.com/ Parameters do not match. {} doesn't"
-            " match {does_not_exist: test}\n"
+            " match {'does_not_exist': 'test'}\n"
             "You can use `strict_match=True` to do a strict parameters check."
         ) in str(exc.value)
 
@@ -304,7 +318,7 @@ def test_request_matches_empty_body():
                 )
 
             msg = str(excinfo.value)
-            assert "request.body doesn't match: {my: data} doesn't match {}" in msg
+            assert "request.body doesn't match: {'my': 'data'} doesn't match {}" in msg
 
         with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps:
             rsps.add(
@@ -322,9 +336,9 @@ def test_request_matches_empty_body():
                 )
             msg = str(excinfo.value)
             assert (
-                "request.body doesn't match: {page: second, type: urlencoded} doesn't match {}"
-                in msg
-            )
+                "request.body doesn't match: {'page': 'second', "
+                "'type': 'urlencoded'} doesn't match {}"
+            ) in msg
 
     run()
     assert_reset()
@@ -389,7 +403,8 @@ def test_fail_matchers_error():
 
             msg = str(excinfo.value)
             assert (
-                "request.body doesn't match: {id: bad} doesn't match {foo: bar}" in msg
+                "request.body doesn't match: {'id': 'bad'} doesn't match {'foo': 'bar'}"
+                in msg
             )
 
             assert (
@@ -418,10 +433,11 @@ def test_fail_matchers_error():
 
             msg = str(excinfo.value)
             assert (
-                "Parameters do not match. {id: bad} doesn't match {my: params}" in msg
+                "Parameters do not match. {'id': 'bad'} doesn't match {'my': 'params'}"
+                in msg
             )
             assert (
-                "request.body doesn't match: {page: two} doesn't match {page: one}"
+                "request.body doesn't match: {'page': 'two'} doesn't match {'page': 'one'}"
                 in msg
             )
 
@@ -442,7 +458,7 @@ def test_fail_matchers_error():
             msg = str(excinfo.value)
             assert (
                 "Arguments don't match: "
-                "{stream: True, verify: True} doesn't match {stream: True, verify: False}"
+                "{'stream': True, 'verify': True} doesn't match {'stream': True, 'verify': False}"
             ) in msg
 
     run()
@@ -587,9 +603,9 @@ def test_query_string_matcher_raises():
 
             msg = str(excinfo.value)
             assert (
-                "Query string doesn't match. {didi: pro, test: 1} doesn't match {didi: pro}"
-                in msg
-            )
+                "Query string doesn't match. {'didi': 'pro', 'test': '1'} "
+                "doesn't match {'didi': 'pro'}"
+            ) in msg
 
     run()
     assert_reset()
@@ -642,8 +658,8 @@ def test_request_header_value_mismatch_raises():
 
         msg = str(excinfo.value)
         assert (
-            "Headers do not match: {Accept: application/xml} doesn't match "
-            "{Accept: application/json}"
+            "Headers do not match: {'Accept': 'application/xml'} doesn't match "
+            "{'Accept': 'application/json'}"
         ) in msg
 
     run()
@@ -665,7 +681,9 @@ def test_request_headers_missing_raises():
             requests.get(url, headers={})
 
         msg = str(excinfo.value)
-        assert ("Headers do not match: {} doesn't match {x-custom-header: foo}") in msg
+        assert (
+            "Headers do not match: {} doesn't match {'x-custom-header': 'foo'}"
+        ) in msg
 
     run()
     assert_reset()
@@ -716,8 +734,8 @@ def test_request_matches_headers_strict_match():
 
         msg = str(excinfo.value)
         assert (
-            "Headers do not match: {Accept: text/plain, Accept-Charset: utf-8} "
-            "doesn't match {Accept: text/plain}"
+            "Headers do not match: {'Accept': 'text/plain', 'Accept-Charset': 'utf-8'} "
+            "doesn't match {'Accept': 'text/plain'}"
         ) in msg
 
     run()
@@ -792,31 +810,6 @@ def test_fragment_identifier_matcher_and_match_querystring():
     assert_reset()
 
 
-def test_matchers_create_key_val_str():
-    """
-    Test that matchers._create_key_val_str does recursive conversion
-    """
-    data = {
-        "my_list": [
-            1,
-            2,
-            "a",
-            {"key1": "val1", "key2": 2, 3: "test"},
-            "!",
-            [["list", "nested"], {"nested": "dict"}],
-        ],
-        1: 4,
-        "test": "val",
-        "high": {"nested": "nested_dict"},
-    }
-    conv_str = matchers._create_key_val_str(data)
-    reference = (
-        "{1: 4, high: {nested: nested_dict}, my_list: [!, 1, 2, [[list, nested], {nested: dict}], "
-        "a, {3: test, key1: val1, key2: 2}], test: val}"
-    )
-    assert conv_str == reference
-
-
 class TestHeaderWithRegex:
     @property
     def url(self):  # type: ignore[misc]
@@ -892,9 +885,9 @@ class TestHeaderWithRegex:
                 session.send(prepped)
             msg = str(excinfo.value)
             assert (
-                "Headers do not match: {Accept: text/plain, Message-Signature: "
-                'signature="123",created=abc} '
-                "doesn't match {Accept: text/plain, Message-Signature: "
+                "Headers do not match: {'Accept': 'text/plain', 'Message-Signature': "
+                """'signature="123",created=abc'} """
+                "doesn't match {'Accept': 'text/plain', 'Message-Signature': "
                 "re.compile('signature=\"\\\\S+\",created=\\\\d+')}"
             ) in msg
 
@@ -921,8 +914,8 @@ class TestHeaderWithRegex:
                 session.send(prepped)
             msg = str(excinfo.value)
             assert (
-                "Headers do not match: {Accept: text/plain, Accept-Charset: utf-8} "
-                "doesn't match {Accept: text/plain, Message-Signature: "
+                "Headers do not match: {'Accept': 'text/plain', 'Accept-Charset': 'utf-8'} "
+                "doesn't match {'Accept': 'text/plain', 'Message-Signature': "
                 "re.compile('signature=\"\\\\S+\",created=\\\\d+')}"
             ) in msg
 
@@ -950,10 +943,9 @@ class TestHeaderWithRegex:
                 session.send(prepped)
             msg = str(excinfo.value)
             assert (
-                "Headers do not match: {Accept: text/plain, Accept-Charset: utf-8, "
-                'Message-Signature: signature="abc",'
-                "created=1243} "
-                "doesn't match {Accept: text/plain, Message-Signature: "
+                "Headers do not match: {'Accept': 'text/plain', 'Accept-Charset': 'utf-8', "
+                """'Message-Signature': 'signature="abc",created=1243'} """
+                "doesn't match {'Accept': 'text/plain', 'Message-Signature': "
                 "re.compile('signature=\"\\\\S+\",created=\\\\d+')}"
             ) in msg
 
diff --git a/responses/tests/test_responses.py b/responses/tests/test_responses.py
index 12176b9..9088ff3 100644
--- a/responses/tests/test_responses.py
+++ b/responses/tests/test_responses.py
@@ -2416,6 +2416,18 @@ class TestShortcuts:
         run()
         assert_reset()
 
+    def test_head_with_content_length(self):
+        @responses.activate
+        def run():
+            headers = {"content-length": "1000"}
+            responses.head("http://example.com/1", status=200, headers=headers)
+            resp = requests.head("http://example.com/1")
+            assert resp.status_code == 200
+            assert resp.headers["Content-Length"] == "1000"
+
+        run()
+        assert_reset()
+
     def test_options(self):
         @responses.activate
         def run():