requests-mock icon indicating copy to clipboard operation
requests-mock copied to clipboard

called_with and called_once_with

Open gardenunez opened this issue 5 years ago • 11 comments

Support m.called_with and m.called_once_with to assert requests calls with specific parameters.

For example:

def call_api(data)
    requests.post("http://some_url.com/end_point", data=data)

def test_call_api():
    data = 'some data'
    url = "http://some_url.com/end_point"
    with requests_mock.Mocker() as mock:
        mock.post(url, status_code=200, json={"some:: "response"})
        call_api(data)
    assert mock.called_with(url, data=data)
    assert mock.called_once_with(url, data=data)

gardenunez avatar May 08 '19 12:05 gardenunez

Hmm, that would be useful. I've ended up re-implementing some version of this in different projects depending on what they needed, for example something that tested that the exact history sequence was correct. However that would still be possible, this would just be really useful as well.

I'd just need to figure out how to refactor the matcher logic that's already there to be used here, because I would want to reuse things like the regexp checks and logic.

Unfortunately the Matcher object contains the responses and so it's non-trivial to make that change.

Suggestions and help welcome.

jamielennox avatar May 21 '19 12:05 jamielennox

But, first, to be clear... there are no defined assertions in the mock library. I have never used assertions on the mock API side. I usually use MagicMock for this. That being said, if request_mock goes this route, these should be assertions, not properties. This would be more consistent with the unittest.mock implementation. With that being said, I think these might be useful as assertions.

import requests
url = 'http://some_url.com/end_point'
send_data = {'some': 'data'}
with Mocker() as request_mock:
    mock_matcher = request_mock.post(url, status_code=201) # This already works
    requests.post(url, json=data)

    # Implementation A; assertions on Mocker instance
    request_mock.assert_called_once()
    request_mock.assert_called_with(url, send_data)
    request_mock.assert_called_once_with(url, send_data)

    # Implementation B; assertions on matcher
    mock_matcher.assert_called_once()
    mock_matcher.assert_called_with(url, send_data)
    mock_matcher.assert_called_once_with()

Both of these can be implemented without conflict. But, implementation B is a lot more straight-forward. Implementation A is a little more difficult as it would require looping through the matchers to find the proper one.

rfportilla avatar Apr 16 '20 14:04 rfportilla

So the thing i've tried to avoid here is an assumption on what test suite is being used. If you're using pytest that assert thing makes sense where unittest/testtools i'd do self.assert(thing).

Based on that i've been happy to write functions that return true/false so that you can do assert mocker.called_once() is just as valid as self.assert(mocker.called_once())

jamielennox avatar Apr 19 '20 04:04 jamielennox

So the thing i've tried to avoid here is an assumption on what test suite is being used. If you're using pytest that assert thing makes sense where unittest/testtools i'd do self.assert(thing).

I am certainly not trying to be snarky or argumentative, but continuing the discussion to understand better. I am not sure why test suite enters into it. I am not totally committed to assertions on the requests-mock side in any case (I have yet to need them). I just don't understand how test suite changes object level asserts. I think of unittest.mock as the example to follow and it has its own asserts to make it easier to write (and follow).

I am happy to help either way. I am passionate about testing and requests-mock has been very useful to me. If you prefer "called_with" and "called_once_with", I am happy to create a PR. I will take your guidance.

rfportilla avatar Apr 19 '20 05:04 rfportilla

I would love to see these functions, and I would also love to see the ability to ensure that a Mocker was never called.

fluffy-critter avatar Jun 02 '20 06:06 fluffy-critter

Hi, Fluffy-critter. I would love to see a specific use-case for this (just to add some weight to this). Do you have one?
Btw, you can check if the mock was called with existing methods: called, called_once, call_count. I think the key qualifier for this feature is the "..._with()".

rfportilla avatar Jun 02 '20 19:06 rfportilla

Yes, specifically I have a library that I want to ensure is managing its caching correctly, or that it isn’t trying to make requests to invalid URLs. Other possible use cases would be things like blacklisting peers or the like.

fluffy-critter avatar Jun 02 '20 20:06 fluffy-critter

It turns out that my use case is better suited using unittest.mock.patch, e.g.:

def test_not_address():
    with unittest.mock.patch('requests.get') as mock:
        assert webfinger.get_profiles("http://example.com") == set()
        assert webfinger.get_profiles("[email protected]") == set()
        assert webfinger.get_profiles("@quux") == set()

        mock.assert_not_called()

fluffy-critter avatar Jun 03 '20 19:06 fluffy-critter

There is certainly more than one way. :-)

This is more of a unit test approach by mocking requests.get. This works well if you are testing a single function and know it will be calling requests.get AND you are not concerned with matching on specific calls. Requests mock allows for more feature test approach by catching the call at the adapter layer. I think this is much better for testing use cases and don't know where or when (or how) a url might be called. It can also protect from calling a live site accidentally when you only mean to test. :-) Well, enough with the sales pitch. haha!

Using requests_mock, I think you could do the above like so:

import requests_mock
def test_not_address():
    lookup_site = 'https://profile_site.com'
    with requests_mock.Mocker() as m:
        m.register_uri(requests_mock.ANY, requests_mock.ANY)  # matches any request
        assert webfinger.get_profiles("http://example.com") == set()
        assert webfinger.get_profiles("[email protected]") == set()
        assert webfinger.get_profiles("@quux") == set()

        assert m.called == False

This is like using a sledgehammer in place of a scalpel. :-D

rfportilla avatar Jun 03 '20 20:06 rfportilla

Oh, is m.called already available from requests_mock? I'm pretty new to the Python unit testing world and trying to learn more about it, and the pytest and unittest docs are something of a rabbit hole. :)

fluffy-critter avatar Jun 03 '20 20:06 fluffy-critter

Back to the point, I think this feature is about being able match on incoming data. This can be done now by passing an "additional_matcher" that checks the request data. This is not straight-forward and has some other implications (1-1 mapping between matcher and incoming data). Therefore, I think there is value in adding some helpers to catch incoming data. I will dig a little deeper here for conjecture. I think that matchers should continue as they are, a collection of a url, a method, and some headers. Matching on incoming data would be a separate function and only focus on the most recent call. This is consistent with unittest.MagicMock.assert_called_with(). @jamielennox, if this seems like the way to go, please confirm and I will try to put some cycles into it when I have a chance.

One last thought: is there a slippery slope here? Requests mock can already match on url, method, headers, and query_string. (am I missing anything?) Data matching would be another thing. Is there anything else that someone will ask about tomorrow? Cookies are discussed in another thread, but are already technically supported in headers.

rfportilla avatar Jun 03 '20 23:06 rfportilla