Unexpected interaction with requests-cache

[bdiv]

Hello and thank you for your work!
We're using requests-cache in our project to reduce load for redundant requests. When we started to use requests_mock we noticed weird behavior. The following minimal working example showcases that:

import pytest
import requests
import requests_cache
import requests_mock

url = "https://www.test.org"
content = "foo"

url2 = "https://www.test.com/testing"
content2 = "bar"

url3 = "https://www.testing.com/test"
content3 = "foobar"

class MyClass:
    def __init__(self, cache=True):
        if cache:
            requests_cache.install_cache("api", backend='memory')

    def query(self, url):
        return requests.get(url)

@pytest.fixture
def mock(requests_mock):
    requests_mock.get(url, text=content)
    return requests_mock

@pytest.fixture
def mock2(requests_mock):
    requests_mock.get(url2, text=content2)
    return requests_mock

@pytest.fixture
def mock3(requests_mock):
    requests_mock.get(url3, text=content3)
    return requests_mock


class Testmytest:
    def test_mock_cache(self, mock):
        assert MyClass(cache=False).query(url).text == content
        assert MyClass(cache=True).query(url).text == content

    def test_mock2_cache(self, mock2):
        assert MyClass(cache=False).query(url2).text == content2
        assert MyClass(cache=True).query(url2).text == content2
    
    def test_mock3_cache(self, mock3):
        assert MyClass(cache=False).query(url3).text == content3
        assert MyClass(cache=True).query(url3).text == content3

We can observe that:

• test_mock_cache does not fail
• test_mock2_cache and test_mock3_cache both fail on the line with cache=True
• the tests fail, because the mock is ignored and the request actually goes through, returning the content of the actual websites

As a workaround we disable caching for our tests. But we still wanted to report this interaction.

[JWCook]

Hi there, requests-cache maintainer here. Similar issues have come up a number of times over here:

• Add compatibility with requests-mock requests-cache/requests-cache#87
• Disabling cache doesn't work with requests_futures requests-cache/requests-cache#135
• Support using requests-cache with other libraries that extend requests.Session requests-cache/requests-cache#158
• AttributeError when using requests_mock.mock() with requests_cache requests-cache/requests-cache#226
• How to patch libraries that make copy of requests.Session() requests-cache/requests-cache#260

There's now a section in the docs on how to safely combine multiple libraries that extend requests, including requests-mock: https://requests-cache.readthedocs.io/en/latest/user_guide/compatibility.html#requests-mock

The solution described there is what requests-cache uses in its own unit tests, since they also happen to use requests-mock.

[jamielennox]

Hmm, that's interesting. I'm really interested in the description on the requests-cache documentation that requests-mock is different. In a way I guess we are, there is no such thing as a RequestsMockSession because you would never use that, the whole point is to not change your code to make this work. We could and maybe should do that so that we become part of the session class chain so that things like this work: https://github.com/reclosedev/requests-cache/blob/79f2dd9707666ca8fb9e63e331d71c0a376c029e/requests_cache/session.py#L156

What i can't wrap my head around at this point in the night is why it still doesn't call us? Technically we are patching requests.Session.send so when requests-cache chains to the parent object's send that should be requests-mock. I assume python runtime stores the mro at some point and patching a class method doesn't update it?

Is it something to do with pytest? The following seems to work:

import requests
import requests_cache
import requests_mock
import unittest

url = "https://www.test.org"
content = "foo"

url2 = "https://www.test.com/testing"
content2 = "bar"

url3 = "https://www.testing.com/test"
content3 = "foobar"


class Testmytest(unittest.TestCase):

    def setUp(self):
        super(Testmytest, self).setUp()
        requests_cache.clear()

    def test_cache_mock_first(self):
        with requests_mock.mock() as m:
            m.get(url2, text=content2)

            with requests_cache.enabled(backend='memory'):
                self.assertEqual(content2, requests.get(url2).text)

    def test_cache_mock_last(self):
        with requests_cache.enabled(backend='memory'):
            with requests_mock.mock() as m:
                m.get(url3, text=content3)
                self.assertEqual(content3, requests.get(url3).text)


if __name__ == '__main__':
    unittest.main()

I know there are a number of requests libraries that all share this space now. I'd like to integrate as nicely as possible so maybe we do refactor this for a new major version, but if there are any ideas as to what's going on here and how to improve it we will.

Can i make a suggestion to @JWCook though, I think you want to have an option to disable requests-cache for running in unit tests. Even trying to figure out what was happening here i found that requests-cache defaulted to writing a http_cache.sqlite file to disk which basically changed the flow of tests depending on ordering. In a unittest situation caching is a bad idea.

[JWCook]

In general, yes, I think disabling requests-cache for unit tests is the best option. There are a few options for that, including:

• session.cache_disabled() (if using CachedSession directly)
• requests_cache.disabled() or uninstall_cache() (if using the patching functions)

Do you have any ideas for making that more convenient specifically for unit tests?

I guess it depends on what a user wants to accomplish in unit tests, though. In both of the tests in your example, CachedSession.send() never actually gets called. That http_cache.sqlite file just gets created initially by requests_cache.enabled() (if using the default SQLite backend), and isn't written to or read from. The _original_send stored in requests_mock.mocker won't apply to a custom session object that's already been created, or if requests.Session is monkey-patched after requests_mock.mocker is imported. Similar situation with the original example.

I don't see that as a big problem, if disabling requests-cache is an acceptable solution. But if for some reason you wanted both request mocking and caching features at the same time (for example, in the unit tests for requests-cache itself), the nested contextmanagers in that example don't do that.

Personally I don't think any changes are needed in requests-mock just for this case, but maybe for the more general case of working with other libraries that modify requests.Session? Particularly ones that actually modify response content (for example, requests-html).

[JWCook]

@jamielennox Fyi, I added some better examples here. The last one is probably the most potentially useful.

Also see requests-cache/requests-cache#330 And let me know if you have any thoughts on that. Thanks!