Introduce Redis integration tests + resolve key type disparity across Redis and Cassandra#284
Introduce Redis integration tests + resolve key type disparity across Redis and Cassandra#284ekzhu merged 24 commits intoekzhu:masterfrom
Redis integration tests + resolve key type disparity across Redis and Cassandra#284Conversation
This reverts commit 36f0ef5.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces Redis integration tests and resolves type inconsistencies between Redis and Cassandra storage backends. The changes parametrize existing Cassandra integration tests to run for both storage types, ensuring feature parity and making it easier to extend tests to new storage layers in the future.
Key changes:
- Converted test suite from
unittest.TestCaseto pytest-style classes with parametrized fixtures - Added encoder/decoder functionality to Redis storage to match Cassandra's string/bytes handling
- Introduced GitHub Actions workflow for automated Redis integration testing
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/test_integration.py | Converted from unittest to pytest with parametrized fixtures for both Redis and Cassandra; added Redis cleanup functionality |
| datasketch/storage.py | Added value encoder/decoder to RedisListStorage for consistent bytes/string handling across storage backends |
| .github/workflows/test-redis.yml | New workflow for automated Redis integration tests triggered after main test workflow |
| pyproject.toml | Added S101 exception for test files to allow pytest assertions |
Comments suppressed due to low confidence (2)
test/test_integration.py:89
- Manual call to
self.teardown_method(None)is unnecessary and incorrect. In pytest, teardown methods are automatically called after each test. This manual call could interfere with proper test cleanup and is not the pytest way of handling teardown. Consider removing this line and relying on pytest's automatic teardown mechanism.
test/test_integration.py:269 - Manual call to
self.teardown_method(None)is unnecessary and incorrect. In pytest, teardown methods are automatically called after each test. This manual call could interfere with proper test cleanup and is not the pytest way of handling teardown. Consider removing this line and relying on pytest's automatic teardown mechanism.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
for now, regardless of stoarge
|
Added: ensure keys are |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Changes: make |
|
Changes:
|
As requested in a previous PR, we add integration tests for the
Redisstorage. Instead of creating a newtest_redis.pyfile as requested, though, we insteadparametrizethe existing integration tests forCassandrato run for bothRedisandCassandra.This allows us to:
In order to carry this out, we did the following:
unittest.TestCaseclasses intopytestclasses; since we're already usingpytest, this is likely a reasonable changeCassandrastore.While carrying this out, a feature disparity was discovered: when
prepickleisFalse, theCassandrastorage returns strings from relevant key getters, whileRedisreturns bytes. In the same manner as the existingCassandrastorage, we introduce encoders and decoders to unify return types across backends.Added after review: