-
Notifications
You must be signed in to change notification settings - Fork 185
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
redis 3.3.0 returns different values than expected causing failing tests in test_fakeredis.py #242
Comments
I think this is really two failures and I can confirm that these are both legitimate (albeit possibly not purposeful) redis-py API changes: Running
Running
Neither of which is called out in the changelog: https://github.com/andymccurdy/redis-py/blob/master/CHANGES Should we open issues in redis-py? |
I think it is most likely this issue from the changelog:
So I think the short-term fix is to change the fakeredis tests to pass the command name in uppercase and to expect the redis 3.3+ behaviour (which should also be the 3.2- behaviour for uppercase command names). Unfortunately this will probably weaken some of the tests because they're ensuring that the raw value (prior to the conversions done by redis-py) exactly matches real redis e.g. the sign of zero returned by zscore. That may prove important if anyone ever extends fakeredis to support other redis frontends like aioredis. |
@bmerry I was thinking that would be the issue as well but it seems not. The changelog does indicate a change in support for lowercase commands but it reads as though it is expanding support for lowercase commands. I can confirm that the case doesn't matter using
It certainly seems like they may have inadvertently removed support for the ping message functionality. Nothing really stands out in the diff as breaking this. I do see them using the message functionality here though: redis/redis-py@3.2.1...3.3.0#diff-085cbe85ef9bc2ad832d79163eb39172R3187 However the using
Using
So if we switch that test to using the uppercase command we can expect |
Excluding health check failures covered by #240, there are still this unexpected results:
The text was updated successfully, but these errors were encountered: