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

Mocket fails when there are Redis calls using hiredis #147

Closed
WisdomPill opened this issue Mar 20, 2021 · 18 comments
Closed

Mocket fails when there are Redis calls using hiredis #147

WisdomPill opened this issue Mar 20, 2021 · 18 comments

Comments

@WisdomPill
Copy link
Contributor

Describe the bug
I have a django based software that makes http requests using requests and uses django-redis to store some data of what it has fetched. I have noticed that if the first network call in a TestCase is for redis then mocket mocks redis, otherwise it mocks only requests.

To Reproduce

import requests
from django.core.cache import cache
from django.test.testcases import TestCase
from mocket import Mocket, mocketize
from mocket.mockhttp import Entry

class TestMocketStrangeBehaviour(TestCase):
    def tearDown(self):
        cache.clear()

    @mocketize
    def test_b_random_url(self):
        url = "http://www.example.com"
        Entry.single_register(Entry.GET, url)

        requests.get(url)

        self.assertTrue(Mocket.has_requests())

    @mocketize
    def test_a_random_url(self):
        url = "http://www.example.com"
        Entry.single_register(Entry.GET, url)

        cache.set("key", "value")

        requests.get(url)

        self.assertTrue(Mocket.has_requests())

will throw an exception

Error
Traceback (most recent call last):
  File "<decorator-gen-2>", line 2, in test_a_random_url
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/mocket/mocket.py", line 627, in wrapper
    t(*args, **kw)
  File "/Users/anas/Projects/random_project/random_app/tests.py", line 34, in test_a_random_url
    cache.set("key", "value")
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/django_redis/cache.py", line 27, in _decorator
    return method(self, *args, **kwargs)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/django_redis/cache.py", line 76, in set
    return self.client.set(*args, **kwargs)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/django_redis/client/default.py", line 156, in set
    return bool(client.set(nkey, nvalue, nx=nx, px=timeout, xx=xx))
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/client.py", line 1801, in set
    return self.execute_command('SET', *pieces)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/client.py", line 898, in execute_command
    conn = self.connection or pool.get_connection(command_name, **options)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 1192, in get_connection
    connection.connect()
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 567, in connect
    self.on_connect()
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 664, in on_connect
    if nativestr(self.read_response()) != 'OK':
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 739, in read_response
    response = self._parser.read_response()
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 470, in read_response
    self.read_from_socket()
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/connection.py", line 427, in read_from_socket
    bufflen = recv_into(self._sock, self._buffer)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/redis/_compat.py", line 75, in recv_into
    return sock.recv_into(*args, **kwargs)
  File "/Users/anas/Projects/random_project/.venv/lib/python3.7/site-packages/mocket/mocket.py", line 264, in recv_into
    return buffer.write(self.read(buffersize))
AttributeError: 'bytearray' object has no attribute 'write'

If you swap names of the test functions it works as expected.

Expected behavior
Choose what to mock with some flag.

Additional context
It happens in macOS and ubuntu, with python3.7 and also 3.8

@mindflayer
Copy link
Owner

Hi @WisdomPill, I am having a look at that. Thanks for the feedback.
Before starting, I must say that usually when testing Django caching you use DummyCache (https://docs.djangoproject.com/en/3.1/topics/cache/#dummy-caching-for-development), but that of course has nothing to do with mocket failing, which has to be fixed. Maybe you could take it as a workaround for now.

@mindflayer
Copy link
Owner

The other thing is: I have to create a dummy project with Django, your script cannot work without it.

@WisdomPill
Copy link
Contributor Author

Hello @mindflayer,
well, I can't use DummyCache because I am using some features of django-redis that are not standard in django cache, like a redis' Lock.
For now my look around is to run a dummy test that is a mere copy of test_b_random_url of the example, so that alphabetically runs before.
I can create a dummy project with the test case in place if needed.

@mindflayer
Copy link
Owner

What version of mocket are you using?
Nothing is failing on my machine.
Screenshot from 2021-03-20 20-09-27

@mindflayer
Copy link
Owner

mindflayer commented Mar 20, 2021

I launched a env DJANGO_SETTINGS_MODULE=app.app.settings pytest test.py
Django's app has been created with a simple django-admin startproject app. Probably it is not failing because I did not configure Django caching.

@mindflayer
Copy link
Owner

mindflayer commented Mar 20, 2021

What redis version are you using? I had problems in the past when the author changed the way it manages connections. I need to isolate the problem, since it's clear that it has nothing to do with Django.

@mindflayer
Copy link
Owner

mindflayer commented Mar 20, 2021

All mocket tests are passing with the latest version of redis:

[...]
----------- coverage: platform linux, python 3.8.6-final-0 -----------
Name                                   Stmts   Miss  Cover   Missing
--------------------------------------------------------------------
mocket/__init__.py                         3      0   100%
mocket/async_mocket.py                    12      2    83%   7, 10
mocket/mocket.py                         392      7    98%   71-73, 87, 102, 189, 309
mocket/mockhttp.py                       124      0   100%
mocket/mockredis.py                       52      0   100%
mocket/plugins/__init__.py                 0      0   100%
mocket/plugins/httpretty/__init__.py      71      0   100%
mocket/plugins/httpretty/core.py           2      0   100%
mocket/plugins/pook_mock_engine.py        43      0   100%
mocket/utils.py                           20      0   100%
--------------------------------------------------------------------
TOTAL                                    719      9    99%

=========================================================================== 140 passed, 12 warnings in 9.71s ===========================================================================

(.venv) drizzt@mindflayer ~/r/python-mocket (master)> pip freeze | grep redis
redis==3.5.3

@WisdomPill
Copy link
Contributor Author

I am using the same redis version.
I am trying to setup a dummy project.
Here is my pipfile with the dependencies needed if you want to try in the meantime.

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[dev-packages]

[packages]
django = "3.1.2"
django-redis = "==4.12.1"
mocket = "==3.9.40"
redis = "==3.5.3"
requests = "==2.24.0"

@mindflayer
Copy link
Owner

I am trying to setup a dummy project.

Thanks, that would help. :)

@WisdomPill
Copy link
Contributor Author

WisdomPill commented Mar 20, 2021

With just the requirements of my previous comment it seems to work, but if I copy the requirements of the project in which there is the issue it happens again.
I stripped down the dependencies to see if the issue is still there and it is.
I left most of the dependencies that have to do with redis.
I will try to remove more dependencies, in the meantime here is the link to the repo, you can have a look at the actions, I can add you to the repo if you want to use the action that is implemented there.
I can also add a matrix to see if with other versions of packages, python or redis it might differ,
but at the moment I don't think so.

@WisdomPill
Copy link
Contributor Author

It has to do with hiredis, I will read further probably tomorrow.
Thanks for the quick response.
I will let you know why it happens and if there is a quick fix to make mocket work in harmony with hiredis

@mindflayer
Copy link
Owner

It has to do with hiredis, I will read further probably tomorrow.

That's quite strange, AFAIK hiredis has nothing to do with networking and it's for speeding up the Redis protocol parsing.

@mindflayer
Copy link
Owner

Let me know in case you feel I can be of help.

@WisdomPill
Copy link
Contributor Author

I did some digging and removed django.
I found out that redis-py gives the error only when hiredis is available because it tries to use the socket to read (have a look here) the response. In fact now it gives the error back all the time regardless of the orther of the tests.
I never used mocket for redis so I do not know how it mocks behind the scenes, can you help me understand why even if using @mocketized I can access redis? I thought pocket will mock all the sockets, in fact that is the reason why the test fails.
I updated the project, maybe you can have a look at it?

@mindflayer
Copy link
Owner

mindflayer commented Mar 22, 2021

can you help me understand why even if using @mocketized I can access redis?

When using mocket, you are literally mocking the socket module with the first acting as a proxy. So, all the calls which are mapped by specific Entry are mocked, all the rest are supposed to "fall-back" to their real sockets. In this case mocket is failing on using the real socket when hiredis is in the middle.

At the moment the alternatives you have are:

  • using mocket as a context manager and mock only your HTTP call as follows;
from mocket import Mocket, Mocketizer

def test_random_url_with_redis(redis_client: Redis):
    url = "http://www.example.com"
    Entry.single_register(Entry.GET, url)

    redis_client.set("key", "value")

    with Mocketizer():
        requests.get(url)
        assert Mocket.has_requests()
  • removing hiredis when testing;
  • mocking all your Redis calls with patch from unittest.mock or any other solution that prevents Python to opening a socket.

I'll do my best to understand how to fix the error, but sometimes it takes a while.

@mindflayer mindflayer changed the title Mocket mocking redis when not needed Mocket fails when there are Redis calls using hiredis Mar 22, 2021
@mindflayer mindflayer added the bug label Mar 22, 2021
@WisdomPill
Copy link
Contributor Author

Okay, thanks for the explanation.
I already found some workaround in my projects, thanks for pointing out some solutions though.
I just like mocket and want to help by reporting this issue.

P.S. I wanted to make another point in why I am not using DummyCache, I prefer having the exact environment of staging and production during testing, so DummyCache is something that I never even thought of using to not incur in any preventable issue.

@mindflayer
Copy link
Owner

mindflayer commented Mar 22, 2021

I understand your point of view and I believe it makes sense, especially if your logic can be different depending on something cached or not, but also consider that a cache, by definition, should not be a "blocker" and your application should be able to work when Redis is not available, it's just there as an additional layer which helps to speed up the flow.
This said, the first alternative I presented to you (context manager) implies you have a working Redis in your testing environment. If so, everything will work properly.

@mindflayer
Copy link
Owner

mindflayer commented Mar 22, 2021

I just like mocket and want to help by reporting this issue.

Thank you so much for your help, btw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants