Repository: redis/redis-py
File: tests/test_maintenance_events_handling.py (Python)

Code before:
        for t in threads:
            t.start()
        for t in threads:
            t.join()
        assert all(results), f"Not all threads succeeded: {results}"
        assert not errors, f"Errors occurred in threads: {errors}"
        # After all threads, MOVING event should have been handled safely
        Helpers.validate_conn_kwargs(
            pool=test_redis_client.connection_pool,
            expected_host_address=AFTER_MOVING_ADDRESS.split(":")[0],
            expected_port=int(DEFAULT_ADDRESS.split(":")[1]),
            expected_socket_timeout=self.config.relax_timeout,
            expected_socket_connect_timeout=self.config.relax_timeout,
            expected_orig_host_address=DEFAULT_ADDRESS.split(":")[0],
            expected_orig_socket_timeout=None,
            expected_orig_socket_connect_timeout=None,
        )

        if hasattr(test_redis_client.connection_pool, "disconnect"):
            test_redis_client.connection_pool.disconnect()

    @pytest.mark.parametrize("pool_class", [ConnectionPool, BlockingConnectionPool])
    def test_moving_migrating_migrated_moved_state_transitions(self, pool_class):
        """
        Test moving configs are not lost if the per connection events get picked up after moving is handled.
        MOVING → MIGRATING → MIGRATED → FAILING_OVER → FAILED_OVER → MOVED
        Checks the state after each event for all connections and for new connections created during each state.
        """
        # Setup
        test_redis_client = self._get_client(
            pool_class, max_connections=5, setup_pool_handler=True
        )
        pool = test_redis_client.connection_pool
        pool_handler = pool.connection_kwargs["maintenance_events_pool_handler"]

        # Create and release some connections
        in_use_connections = []
        for _ in range(3):
            in_use_connections.append(pool.get_connection())
        while len(in_use_connections) > 0:
            pool.release(in_use_connections.pop())

        # Take 2 connections to be in use
        in_use_connections = []
        for _ in range(2):
            conn = pool.get_connection()
            in_use_connections.append(conn)

        # 1. MOVING event
        tmp_address = "22.23.24.25"
        moving_event = NodeMovingEvent(

Review comment (bug):
The comment describes a sequence that doesn't match the actual test behavior. The test shows that FAILING_OVER and FAILED_OVER events don't change the connection state when it's already in MOVING state, so the arrow notation is misleading.
```suggestion
        Sequence of events: MOVING, MIGRATING, MIGRATED, FAILING_OVER, FAILED_OVER, MOVED.
        Note: FAILING_OVER and FAILED_OVER events do not change the connection state when already in MOVING state.
```

Code after:
        for t in threads:
            t.start()
        for t in threads:
            t.join()
        assert all(results), f"Not all threads succeeded: {results}"
        assert not errors, f"Errors occurred in threads: {errors}"
        # After all threads, MOVING event should have been handled safely
        Helpers.validate_conn_kwargs(
            pool=test_redis_client.connection_pool,
            expected_host_address=AFTER_MOVING_ADDRESS.split(":")[0],
            expected_port=int(DEFAULT_ADDRESS.split(":")[1]),
            expected_socket_timeout=self.config.relax_timeout,
            expected_socket_connect_timeout=self.config.relax_timeout,
            expected_orig_host_address=DEFAULT_ADDRESS.split(":")[0],
            expected_orig_socket_timeout=None,
            expected_orig_socket_connect_timeout=None,
        )

        if hasattr(test_redis_client.connection_pool, "disconnect"):
            test_redis_client.connection_pool.disconnect()

    @pytest.mark.parametrize("pool_class", [ConnectionPool, BlockingConnectionPool])
    def test_moving_migrating_migrated_moved_state_transitions(self, pool_class):
        """
        Test moving configs are not lost if the per connection events get picked up after moving is handled.
        Sequence of events: MOVING, MIGRATING, MIGRATED, FAILING_OVER, FAILED_OVER, MOVED.
        Note: FAILING_OVER and FAILED_OVER events do not change the connection state when already in MOVING state.
        Checks the state after each event for all connections and for new connections created during each state.
        """
        # Setup
        test_redis_client = self._get_client(
            pool_class, max_connections=5, setup_pool_handler=True
        )
        pool = test_redis_client.connection_pool
        pool_handler = pool.connection_kwargs["maintenance_events_pool_handler"]

        # Create and release some connections
        in_use_connections = []
        for _ in range(3):
            in_use_connections.append(pool.get_connection())
        while len(in_use_connections) > 0:
            pool.release(in_use_connections.pop())

        # Take 2 connections to be in use
        in_use_connections = []
        for _ in range(2):
            conn = pool.get_connection()
            in_use_connections.append(conn)

        # 1. MOVING event
        tmp_address = "22.23.24.25"