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

Code before:
        assert len(res) > 2
        assert "prefixes" in res or b"prefixes" in res

    @skip_if_server_version_lt("6.0.0")
    @skip_if_redis_enterprise()
    def test_client_tracking(self, r):
        # simple case - will execute on all node
        assert r.client_tracking_on()
        assert r.client_tracking_off()

        # id based
        node = r.get_default_node()
        # when id is provided - the command should be sent to the node that
        # owns the connection with this id
        client_id = node.redis_connection.client_id()
        assert r.client_tracking_on(clientid=client_id, target_nodes=node)
        assert r.client_tracking_off(clientid=client_id, target_nodes=node)

        # execute with client id and prefixes and bcast
        assert r.client_tracking_on(
            clientid=client_id, prefix=["foo", "bar"], bcast=True, target_nodes=node
        )

        # now with some prefixes and without bcast
        with pytest.raises(DataError):
            assert r.client_tracking_on(prefix=["foo", "bar", "blee"])

    @skip_if_server_version_lt("2.9.50")
    def test_client_pause(self, r):
        node = r.get_primaries()[0]
        assert r.client_pause(1, target_nodes=node)
        assert r.client_pause(timeout=1, target_nodes=node)
        with pytest.raises(RedisError):
            r.client_pause(timeout="not an integer", target_nodes=node)

    @skip_if_server_version_lt("6.2.0")
    @skip_if_redis_enterprise()
    def test_client_unpause(self, r):
        assert r.client_unpause()

    @skip_if_server_version_lt("5.0.0")
    def test_client_id(self, r):
        node = r.get_primaries()[0]
        assert r.client_id(target_nodes=node) > 0

    @skip_if_server_version_lt("5.0.0")
    def test_client_unblock(self, r):
        node = r.get_primaries()[0]
        myid = r.client_id(target_nodes=node)
        assert not r.client_unblock(myid, target_nodes=node)
        assert not r.client_unblock(myid, error=True, target_nodes=node)

Review comment (suggestion):
The `assert` statement here is unnecessary and misleading. When `pytest.raises(DataError)` is used, we're only checking that a `DataError` exception is raised by the enclosed code. The return value (if any) is not checked. Since `client_tracking_on()` will raise a `DataError` before returning (due to prefix being set without bcast), this assert will never be evaluated. Consider removing the `assert` to make the intent clearer:
```python
with pytest.raises(DataError):
    r.client_tracking_on(prefix=["foo", "bar", "blee"])
```
```suggestion
            r.client_tracking_on(prefix=["foo", "bar", "blee"])
```

Code after:
        assert len(res) > 2
        assert "prefixes" in res or b"prefixes" in res

    @skip_if_server_version_lt("6.0.0")
    @skip_if_redis_enterprise()
    def test_client_tracking(self, r):
        # simple case - will execute on all nodes
        assert r.client_tracking_on()
        assert r.client_tracking_off()

        # id based
        node = r.get_default_node()
        # when id is provided - the command should be sent to the node that
        # owns the connection with this id
        client_id = node.redis_connection.client_id()
        assert r.client_tracking_on(clientid=client_id, target_nodes=node)
        assert r.client_tracking_off(clientid=client_id, target_nodes=node)

        # execute with client id and prefixes and bcast
        assert r.client_tracking_on(
            clientid=client_id, prefix=["foo", "bar"], bcast=True, target_nodes=node
        )

        # now with some prefixes and without bcast
        with pytest.raises(DataError):
            assert r.client_tracking_on(prefix=["foo", "bar", "blee"])

    @skip_if_server_version_lt("2.9.50")
    def test_client_pause(self, r):
        node = r.get_primaries()[0]
        assert r.client_pause(1, target_nodes=node)
        assert r.client_pause(timeout=1, target_nodes=node)
        with pytest.raises(RedisError):
            r.client_pause(timeout="not an integer", target_nodes=node)

    @skip_if_server_version_lt("6.2.0")
    @skip_if_redis_enterprise()
    def test_client_unpause(self, r):
        assert r.client_unpause()

    @skip_if_server_version_lt("5.0.0")
    def test_client_id(self, r):
        node = r.get_primaries()[0]
        assert r.client_id(target_nodes=node) > 0

    @skip_if_server_version_lt("5.0.0")
    def test_client_unblock(self, r):
        node = r.get_primaries()[0]
        myid = r.client_id(target_nodes=node)
        assert not r.client_unblock(myid, target_nodes=node)
        assert not r.client_unblock(myid, error=True, target_nodes=node)