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

Code before:
        # Verify we got messages from both channels
        channels_received = {msg["channel"] for msg in messages}
        assert channel1.encode() in channels_received
        assert channel2.encode() in channels_received

        pubsub.close()

    @pytest.mark.onlycluster
    @skip_if_server_version_lt("7.0.0")
    def test_generator_handles_concurrent_mapping_changes(self, r):
        """
        Test that the generator properly handles mapping changes during iteration.
        This validates the fix for the RuntimeError: dictionary changed size during iteration.
        """
        pubsub = r.pubsub()
        channel1 = "test-channel:{0}"
        channel2 = "test-channel:{6}"

        # Subscribe to first channel
        pubsub.ssubscribe(channel1)
        msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
        assert msg is not None
        assert msg["type"] == "ssubscribe"

        # Get initial mapping size (if available)
        initial_size = 0
        if hasattr(pubsub, "node_pubsub_mapping"):
            initial_size = len(pubsub.node_pubsub_mapping)

        # Subscribe to second channel (modifies mapping during potential iteration)
        pubsub.ssubscribe(channel2)
        msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
        assert msg is not None
        assert msg["type"] == "ssubscribe"

        # Verify mapping was updated (if available)
        if hasattr(pubsub, "node_pubsub_mapping"):
            assert len(pubsub.node_pubsub_mapping) >= initial_size

        # Publish and read messages - should not raise RuntimeError
        r.spublish(channel1, "msg1")
        r.spublish(channel2, "msg2")

        messages_received = 0
        for _ in range(2):
            msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
            if msg and msg["type"] == "smessage":
                messages_received += 1

        assert messages_received == 2
        pubsub.close()

Review comment (suggestion):
Initializing `initial_size` to 0 when the attribute doesn't exist makes the subsequent comparison meaningless. If `node_pubsub_mapping` is a required attribute for cluster pubsub, remove the hasattr check. Otherwise, skip the size validation entirely when the attribute is absent rather than comparing against a meaningless default.
```suggestion

```

Code after:
        # Verify we got messages from both channels
        channels_received = {msg["channel"] for msg in messages}
        assert channel1.encode() in channels_received
        assert channel2.encode() in channels_received

        pubsub.close()

    @pytest.mark.onlycluster
    @skip_if_server_version_lt("7.0.0")
    def test_generator_handles_concurrent_mapping_changes(self, r):
        """
        Test that the generator properly handles mapping changes during iteration.
        This validates the fix for the RuntimeError: dictionary changed size during iteration.
        """
        pubsub = r.pubsub()
        channel1 = "test-channel:{0}"
        channel2 = "test-channel:{6}"

        # Subscribe to first channel
        pubsub.ssubscribe(channel1)
        msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
        assert msg is not None
        assert msg["type"] == "ssubscribe"

        # Get initial mapping size (cluster pubsub only)
        assert hasattr(pubsub, "node_pubsub_mapping"), "Test requires ClusterPubSub"
        initial_size = len(pubsub.node_pubsub_mapping)

        # Subscribe to second channel (modifies mapping during potential iteration)
        pubsub.ssubscribe(channel2)
        msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
        assert msg is not None
        assert msg["type"] == "ssubscribe"

        # Verify mapping was updated
        assert len(pubsub.node_pubsub_mapping) >= initial_size

        # Publish and read messages - should not raise RuntimeError
        r.spublish(channel1, "msg1")
        r.spublish(channel2, "msg2")

        messages_received = 0
        for _ in range(2):
            msg = wait_for_message(pubsub, timeout=1.0, func=pubsub.get_sharded_message)
            if msg and msg["type"] == "smessage":
                messages_received += 1

        assert messages_received == 2
        pubsub.close()