-
Notifications
You must be signed in to change notification settings - Fork 250
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
Mock redis #79
Mock redis #79
Conversation
@@ -14,6 +16,7 @@ | |||
|
|||
class SnapPassTestCase(TestCase): | |||
|
|||
@patch('redis.client.StrictRedis', mock_strict_redis_client) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need to be patched?
It would be nice not to do this because it's the only place the mock
package is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I much prefer mocking third party services like redis than perform an integration test.
The biggest pro for me is that this allows for local development without having to include Redis. We can still add an integration test with redis on travis if we're still interested.
As for introducing the mock
package, it's not ideal but I personally don't think that cost is too high.
Happy to discuss this further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand. The patching is used here because there's no another way for this test case to always use mock_strict_redis_client
should some kind of dependency injection, etc. when MOCK_REDIS
isn't defined. Is that right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, because of how the client is initialized in the main logic, this is the only way I found to replace all instances of the redis_client
with the mock one.
Main upgrade
Now that we've deprecated support for python 3.3 and python 2.6, we can use a compatible library to mock redis.
I looked into using redislite, but the
client.setex
function has a different signature which is incompatible, and required a bit of ugly gluing together.I also tried mockredis and I'm much happier with the simple syntax.
One of the only downsides is that we have to expire keys automatically with a
client.do_expire()
in the tests, but I think this is acceptable.Note that it's still possible to perform integration tests with a real redis, simply do not set the
MOCK_REDIS
ENV variableSide upgrades
Upgraded tox, pytest and pytest cov to use the latest versions