Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calls info in BaseResponse #664

Merged
merged 17 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
0.24.0
------

* Added `BaseResponse.calls` to access calls data of a separate mocked request. See #664
* Added support for re.Pattern based header matching.
* Added support for gzipped response bodies to `json_params_matcher`.
* Moved types-pyyaml dependency to `tests_requires`
Expand Down
69 changes: 69 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,75 @@ Assert that the request was called exactly n times.
responses.assert_call_count("http://www.example.com?hello=world", 1) is True


Assert Request Calls data
------------------

``Request`` object has ``calls`` list which elements correspond to ``Call`` objects
in the global list of ``Registry``. This can be useful when the order of requests is not
guaranteed, but you need to check their correctness, for example in multithreaded
applications.

.. code-block:: python

import concurrent.futures
import responses
import requests


@responses.activate
def test_assert_calls_on_resp():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that this example might look very close to what you have in production. But can we unload it a bit and reduce complexity?

we probably still want to show an example of concurrent since this could be one of the main usages, but we do not need all those complicated matrices of settings to show the usage

can you please simplify it to remove additional arguments and have more clear names like
rsp1, rsp2, rsp3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you, I'll try to simplify this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplified, please take a look

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeezdev now looks a way better, thank you!

uid_with_permissions = [
("0123", {"client": True, "admin": False, "disabled": False}),
("1234", {"client": False, "admin": True, "disabled": False}),
("2345", {"client": False, "admin": False, "disabled": True}),
]
rsp0123 = responses.patch(
"http://www.foo.bar/0123/",
json={"OK": True, "uid": "0123"},
status=200,
)
rsp1234 = responses.patch(
"http://www.foo.bar/1234/",
json={"OK": False, "uid": "1234"},
status=400,
)
rsp2345 = responses.patch(
"http://www.foo.bar/2345/",
json={"OK": True, "uid": "2345"},
status=200,
)

def update_permissions(uid, permissions):
url = f"http://www.foo.bar/{uid}/"
response = requests.patch(url, json=permissions)
return response

with concurrent.futures.ThreadPoolExecutor(max_workers=3) as executor:
future_to_uid = {
executor.submit(update_permissions, uid, permissions): uid
for (uid, permissions) in uid_with_permissions
}
for future in concurrent.futures.as_completed(future_to_uid):
uid = future_to_uid[future]
response = future.result()
print("%s updated with %d status code" % (uid, response.status_code))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please update it with f-string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


assert len(responses.calls) == 3 # total calls count

assert rsp0123.call_count == 1
assert rsp0123.calls[0] in responses.calls
assert json.loads(rsp0123.calls[0].request.body) == {"client": True, "admin": False, "disabled": False}

assert rsp1234.call_count == 1
assert rsp1234.calls[0] in responses.calls
assert json.loads(rsp1234.calls[0].request.body) == {"client": False, "admin": True, "disabled": False}
assert rsp1234.calls[0].response.json() == {"OK": False, "uid": "1234"}

assert rsp2345.call_count == 1
assert rsp2345.calls[0] in responses.calls
assert json.loads(rsp2345.calls[0].request.body) == {"client": False, "admin": False, "disabled": True}


Multiple Responses
------------------

Expand Down
23 changes: 18 additions & 5 deletions responses/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ def __getitem__(self, idx: Union[int, slice]) -> Union[Call, List[Call]]:
def add(self, request: "PreparedRequest", response: _Body) -> None:
self._calls.append(Call(request, response))

def add_call(self, call: Call) -> None:
self._calls.append(call)

def reset(self) -> None:
self._calls = []

Expand Down Expand Up @@ -392,7 +395,7 @@ def __init__(
)

self.match: "_MatcherIterable" = match
self.call_count: int = 0
self._calls: CallList = CallList()
self.passthrough = passthrough

def __eq__(self, other: Any) -> bool:
Expand Down Expand Up @@ -501,6 +504,14 @@ def matches(self, request: "PreparedRequest") -> Tuple[bool, str]:

return True, ""

@property
def call_count(self) -> int:
return len(self._calls)

@property
def calls(self) -> CallList:
return self._calls


def _form_response(
body: Union[BufferedReader, BytesIO],
Expand Down Expand Up @@ -1062,14 +1073,16 @@ def _on_request(
request, match.get_response(request)
)
except BaseException as response:
match.call_count += 1
self._calls.add(request, response)
call = Call(request, response)
self._calls.add_call(call)
match.calls.add_call(call)
raise

if resp_callback:
response = resp_callback(response) # type: ignore[misc]
match.call_count += 1
self._calls.add(request, response) # type: ignore[misc]
call = Call(request, response)
self._calls.add_call(call)
match.calls.add_call(call)

retries = retries or adapter.max_retries
# first validate that current request is eligible to be retried.
Expand Down
83 changes: 83 additions & 0 deletions responses/tests/test_responses.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import inspect
import json
import os
import re
import warnings
Expand Down Expand Up @@ -1986,6 +1987,88 @@ def run():
assert_reset()


def test_response_and_requests_mock_calls_are_equal():
@responses.activate
def run():
rsp = responses.add(responses.GET, "http://www.example.com")
rsp2 = responses.add(responses.GET, "http://www.example.com/1")

requests.get("http://www.example.com")
requests.get("http://www.example.com/1")

assert len(responses.calls) == 2
assert rsp.call_count == 1
assert rsp.calls[0] is responses.calls[0]
assert rsp2.call_count == 1
assert rsp2.calls[0] is responses.calls[1]

run()
assert_reset()


def test_response_call_request():
@responses.activate
def run():
rsp = responses.add(responses.GET, "http://www.example.com")
rsp2 = responses.add(
responses.PUT, "http://www.foo.bar/42/", json={"id": 42, "name": "Bazz"}
)

requests.get("http://www.example.com")
requests.get("http://www.example.com?hello=world")
requests.put(
"http://www.foo.bar/42/",
json={"name": "Bazz"},
)

assert rsp.call_count == 2
request = rsp.calls[0].request
assert request.url == "http://www.example.com/"
assert request.method == "GET"
request = rsp.calls[1].request
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make it a bit more readable by enumerating request objects as well

Suggested change
assert rsp.call_count == 2
request = rsp.calls[0].request
assert request.url == "http://www.example.com/"
assert request.method == "GET"
request = rsp.calls[1].request
assert rsp.call_count == 2
request1 = rsp.calls[0].request
assert request.url == "http://www.example.com/"
assert request.method == "GET"
request2 = rsp.calls[1].request

and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense. Made tests readable. Please take a look

assert request.url == "http://www.example.com/?hello=world"
assert request.method == "GET"
assert rsp2.call_count == 1
request = rsp2.calls[0].request
assert request.url == "http://www.foo.bar/42/"
assert request.method == "PUT"
request_payload = json.loads(request.body)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exactly do we test here ?

why do we need to test that request body could be converted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I changed the test a little)

Here we validate that request rsp2_request1 = rsp2.calls[0].request obtained from the mock's call contains expected payload request_payload = {"name": "Bazz"}, that was used on the put request.

request body could be converted?

PreparedRequest contains only encoded data in the body field, so we decode it to compare with the original data - request_payload.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PreparedRequest contains only encoded data in the body field, so we decode it to compare with the original data - request_payload.

correct, but do we need to check it at all as part of responses ?

we attach request object as is without modification

response.request = request

and in the case of this PR we can skip this check since it does not add a value.

although, you can fire a parallel PR that can add a separate test to validate that we have all the required data is attached to a request
we still will need to discuss if that is required or not but this we can do in a separate thread

Copy link
Contributor Author

@zeezdev zeezdev Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we attach request object as is without modification

You are right, I missed this moment.

So, in other words, in the current PR it is enough to test that calls of local mocks are identical to calls of the global list without deep validation of rsp.calls[N].request & rsp.calls[N].response?
I.e. merge all new tests (test_response_and_requests_mock_calls_are_equal, test_response_call_request, test_response_call_response) to something like this:

def test_response_calls_and_registry_calls_are_equal():
    @responses.activate
    def run():
        rsp1 = responses.add(responses.GET, "http://www.example.com")
        rsp2 = responses.add(responses.GET, "http://www.example.com/1")

        requests.get("http://www.example.com")
        requests.get("http://www.example.com/1")
        requests.get("http://www.example.com")

        assert len(responses.calls) == len(rsp1.calls) + len(rsp2.calls)
        assert rsp1.call_count == 2
        assert len(rsp1.calls) == 2
        assert rsp1.calls[0] is responses.calls[0]
        assert rsp1.calls[1] is responses.calls[2]
        assert rsp2.call_count == 1
        assert len(rsp2.calls) == 1
        assert rsp2.calls[0] is responses.calls[1]

    run()
    assert_reset()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@beliaev-maksim and what about this question?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeezdev sorry, missed this one in a long thread. Yes.
And if for some reason we have missed the tests that ensure the quality of Call objects. Then I would recommend to create separate tests for those and open it in a separate PR.

assert request_payload == {"name": "Bazz"}

run()
assert_reset()


def test_response_call_response():
@responses.activate
def run():
rsp = responses.add(responses.GET, "http://www.example.com", body=b"test")
rsp2 = responses.add(
responses.POST,
"http://www.foo.bar/42/",
json={"id": 42, "name": "Bazz"},
status=201,
)

requests.get("http://www.example.com")
requests.post(
"http://www.foo.bar/42/",
json={"name": "Bazz"},
)

assert rsp.call_count == 1
response = rsp.calls[0].response
assert response.content == b"test"
assert response.status_code == 200
assert rsp2.call_count == 1
response = rsp2.calls[0].response
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, could be enumerated. Something like

Suggested change
response = rsp2.calls[0].response
response2 = rsp2.calls[0].response

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, done

assert response.json() == {"id": 42, "name": "Bazz"}
assert response.status_code == 201

run()
assert_reset()


def test_fail_request_error():
"""
Validate that exception is raised if request URL/Method/kwargs don't match
Expand Down