-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix type hints for register_script to support RedisCluster #3876
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
base: master
Are you sure you want to change the base?
Changes from all commits
a70fa15
3baf342
ffe8733
abb99a5
b162a62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import pytest | ||
| import redis | ||
| from redis import exceptions | ||
| from redis.cluster import RedisCluster | ||
| from redis.commands.core import Script | ||
| from tests.conftest import skip_if_redis_enterprise, skip_if_server_version_lt | ||
|
|
||
|
|
@@ -53,6 +54,24 @@ def test_encoder(self, r, script_bytes): | |
| assert encoder is not None | ||
| assert encoder.encode("fake-script") == b"fake-script" | ||
|
|
||
| def test_script_with_cluster_client(self, script_bytes): | ||
| """Test that Script class accepts RedisCluster as registered_client. | ||
|
|
||
| This verifies the type hints fix for register_script to support RedisCluster. | ||
| We use a mock-like approach since we don't need actual cluster connection. | ||
| Using bytes script to avoid encoder dependency in mock. | ||
| """ | ||
| from unittest.mock import MagicMock | ||
|
|
||
| # Create a mock RedisCluster instance | ||
| mock_cluster = MagicMock(spec=RedisCluster) | ||
|
|
||
| # Script should accept RedisCluster without type errors | ||
| # Using bytes script to bypass encoder.encode() call | ||
| script = Script(mock_cluster, script_bytes) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to validate that you can actually register the script using the RedisCluster client. With unit tests you won't be able to validate that we don't have a static code analyses errors, but what we can do is to validate that by providing the RedisCluster as valid input type - the client actually can be used to execute a register_script call. |
||
| assert script.registered_client is mock_cluster | ||
| assert script.script == script_bytes | ||
|
|
||
|
|
||
| class TestScripting: | ||
| @pytest.fixture(autouse=True) | ||
|
|
||
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.
Import should be added at the top of the file.