Add e2e tests - validate new connections will receive maintenance notifications#3921
Merged
petyaslavova merged 4 commits intofeat/hitless_upgrade_sync_cluster_clientfrom Jan 29, 2026
Conversation
petyaslavova
changed the base branch from
master
to
feat/hitless_upgrade_sync_cluster_client
Contributor
There was a problem hiding this comment.
Pull request overview
Adds/extends end-to-end and unit test coverage to ensure maintenance push notifications are delivered (including to newly created connections), and introduces Redis OSS Cluster (SMIGRATING/SMIGRATED) maintenance notification support with a proxy-based test harness.
Changes:
- Expand scenario/e2e tests for maintenance notifications, including “new connections receive notifications” cases and cluster scenarios.
- Add OSS Cluster maintenance notification parsing/handling (SMIGRATING/SMIGRATED) and wire handlers through connection pools, parsers, and RedisCluster.
- Add a proxy-based test environment (docker-compose service + helper utilities) to simulate cluster topology/notifications.
Reviewed changes
Copilot reviewed 61 out of 62 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_scenario/test_maint_notifications.py | Refactors and expands scenario tests; adds standalone vs cluster coverage and additional flows. |
| tests/test_scenario/test_active_active.py | Type annotation tweak for optional threading.Event. |
| tests/test_scenario/maint_notifications_helpers.py | Adds cluster-aware helpers for waiting on/releasing connections and key generation utilities. |
| tests/test_scenario/fault_injector_client.py | Introduces abstract FI interface + implementations (RE + proxy) to support new scenarios/effects. |
| tests/test_scenario/conftest.py | Adds cluster endpoint config fixtures and toggling between real RE and mock proxy environments. |
| tests/test_asyncio/test_scenario/conftest.py | Updates asyncio scenario FI client to use REFaultInjector. |
| tests/maint_notifications/test_maint_notifications_handling.py | Docstring correction for return value. |
| tests/maint_notifications/test_maint_notifications.py | Adds unit tests for OSS notification classes and updates handler call signatures. |
| tests/maint_notifications/test_cluster_maint_notifications_handling.py | New comprehensive test suite for cluster maintenance notification behavior (proxy-based). |
| tests/maint_notifications/proxy_server_helpers.py | New proxy helper utilities for topology interception and notification injection. |
| tests/conftest.py | Adds CLI option for selecting an OSS cluster endpoint config. |
| redis/utils.py | Adds check_protocol_version() helper to centralize RESP version checks. |
| redis/maint_notifications.py | Adds OSS notification types + cluster handler and extends connection handler behavior. |
| redis/event.py | Uses check_protocol_version() for RESP3 gating. |
| redis/connection.py | Wires OSS cluster maintenance handlers into connections/pools and adds notification tracking fields. |
| redis/cluster.py | Adds maint notification config plumbing and supports reinitialize with additional startup nodes. |
| redis/client.py | Allows passing OSS cluster maintenance handler into ConnectionPool creation. |
| redis/asyncio/cluster.py | Fixes slot detection check (slot is None). |
| redis/_parsers/socket.py | Fixes bug using sock parameter instead of self._sock. |
| redis/_parsers/resp3.py | Adds oss_cluster_maint_push_handler_func field. |
| redis/_parsers/hiredis.py | Adds oss_cluster_maint_push_handler_func field. |
| redis/_parsers/base.py | Adds parsing for SMIGRATING/SMIGRATED and handler dispatch wiring. |
| docker-compose.yml | Adds proxy-based services/network to support local proxy-driven tests. |
| .gitignore | Ignores /experimenting/. |
Comments suppressed due to low confidence (1)
tests/test_cluster.py:1052
- Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| res = cluster.get("test") | ||
| assert res == b"VAL" | ||
| finally: | ||
| cluster.close() |
There was a problem hiding this comment.
Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
| res = cluster.get("test") | ||
| assert res == b"VAL" | ||
| finally: | ||
| cluster.close() |
There was a problem hiding this comment.
Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
| res = cluster.get("test") | ||
| assert res == b"VAL" | ||
| finally: | ||
| cluster.close() |
There was a problem hiding this comment.
Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
| res = cluster.get("test") | ||
| assert res == b"VAL" | ||
| finally: | ||
| cluster.close() |
There was a problem hiding this comment.
Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
| assert results[2] == b"value1" # GET returns value | ||
| assert results[3] == b"value2" # GET returns value | ||
| finally: | ||
| cluster.close() |
There was a problem hiding this comment.
Instance of context-manager class RedisCluster is closed in a finally block. Consider using 'with' statement.
petyaslavova
force-pushed
the
feat/hitless_upgrade_sync_cluster_client
branch
from
9be81e6 to
0728f3c
Compare
petyaslavova
force-pushed
the
add_tests_for_new_connections
branch
2 times, most recently
from
01f07dc to
4d06bbb
Compare
petyaslavova
force-pushed
the
feat/hitless_upgrade_sync_cluster_client
branch
from
0728f3c to
56321af
Compare
…gration with testing helper proxy server. (#3844) * Adding maint_notifications_config to RedisCluster's NodesManager * Adding Redis Proxy integration * Migrating test proxy helper to use custom https client
* Adding SMIGRATED handling * Applying Copilot's comments * Applying review comments
…ster db is now created for every test.
petyaslavova
force-pushed
the
add_tests_for_new_connections
branch
from
4d06bbb to
ff3b593
Compare
elena-kolevska
approved these changes
Jan 29, 2026
petyaslavova
merged commit e07381c
into
feat/hitless_upgrade_sync_cluster_client
2 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
Please provide a description of the change here.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.