Make sure the CI actually runs RESP3 tests (redis#3270)

The CI tests were not running with RESP3 protocol, it was just an
illusion that they do. Fix this, and also preserve coverage and test
artifacts from those runs too.

Some issues have surfaced after the change.

The most notable issue is a bug in hiredis-py, which prevents it
from being used in cluster mode at least. Make sure cluster tests do
not run with hiredis-py. Also make sure some specific unit tests do
not run with hiredis-py.

One other issue with hiredis-py is fixed in this commit. Use a
sentinel object instance to signal lack of data in hiredis-py, instead
of piggybacking of `False`, which can also be returned by parsing
valid RESP payloads.

Some of the unit tests, mostly for modules, were failing, they are now
updated so that they pass.

Remove async parser from test fixture params. Leave the decision for
the async parser to be used in tests to be taken based on the availability
of hiredis-py, and on the protocol that is set for the tests. Otherwise
when hiredis-py is available we would also run the non-hiredis tests.

Signed-off-by: Salvatore Mesoraca <[email protected]>

Author: Gabriel Erzse

.github/workflows/integration.yaml

@@ -113,7 +113,9 @@ jobs:
         python-version: ['3.8', '3.11']
         test-type: ['standalone', 'cluster']
         connection-type: ['hiredis', 'plain']
-        protocol: ['3']
+        exclude:
+          - test-type: 'cluster'
+            connection-type: 'hiredis'
     env:
        ACTIONS_ALLOW_UNSECURE_COMMANDS: true
     name: RESP3 [${{ matrix.python-version }} ${{matrix.test-type}}-${{matrix.connection-type}}]
@@ -132,9 +134,33 @@ jobs:
             pip install hiredis
           fi
           invoke devenv
-          sleep 5 # time to settle
-          invoke ${{matrix.test-type}}-tests
-          invoke ${{matrix.test-type}}-tests --uvloop
+          sleep 10 # time to settle
+          invoke ${{matrix.test-type}}-tests --protocol=3
+          invoke ${{matrix.test-type}}-tests --uvloop --protocol=3
+
+      - uses: actions/upload-artifact@v4
+        if: success() || failure()
+        with:
+          name: pytest-results-${{matrix.test-type}}-${{matrix.connection-type}}-${{matrix.python-version}}-resp3
+          path: '${{matrix.test-type}}*results.xml'
+
+      - name: Upload codecov coverage
+        uses: codecov/codecov-action@v4
+        with:
+          fail_ci_if_error: false
+
+      - name: View Test Results
+        uses: dorny/test-reporter@v1
+        if: success() || failure()
+        continue-on-error: true
+        with:
+          name: Test Results ${{matrix.python-version}} ${{matrix.test-type}}-${{matrix.connection-type}}-resp3
+          path: '*.xml'
+          reporter: java-junit
+          list-suites: all
+          list-tests: all
+          max-annotations: 10
+          fail-on-error: 'false'
 
    build_and_test_package:
     name: Validate building and installing the package

tests/test_asyncio/conftest.py

@@ -5,13 +5,11 @@
 import pytest_asyncio
 import valkey.asyncio as valkey
 from tests.conftest import VALKEY_INFO
-from valkey._parsers import _AsyncHiredisParser, _AsyncRESP2Parser
 from valkey.asyncio import Sentinel
 from valkey.asyncio.client import Monitor
 from valkey.asyncio.connection import Connection, parse_url
 from valkey.asyncio.retry import Retry
 from valkey.backoff import NoBackoff
-from valkey.utils import HIREDIS_AVAILABLE
 
 from .compat import mock
 
@@ -26,41 +24,21 @@ async def _get_info(valkey_url):
 @pytest_asyncio.fixture(
     params=[
         pytest.param(
-            (True, _AsyncRESP2Parser),
+            (True,),
             marks=pytest.mark.skipif(
                 'config.VALKEY_INFO["cluster_enabled"]', reason="cluster mode enabled"
             ),
         ),
-        (False, _AsyncRESP2Parser),
-        pytest.param(
-            (True, _AsyncHiredisParser),
-            marks=[
-                pytest.mark.skipif(
-                    'config.VALKEY_INFO["cluster_enabled"]',
-                    reason="cluster mode enabled",
-                ),
-                pytest.mark.skipif(
-                    not HIREDIS_AVAILABLE, reason="hiredis is not installed"
-                ),
-            ],
-        ),
-        pytest.param(
-            (False, _AsyncHiredisParser),
-            marks=pytest.mark.skipif(
-                not HIREDIS_AVAILABLE, reason="hiredis is not installed"
-            ),
-        ),
+        (False,),
     ],
     ids=[
-        "single-python-parser",
-        "pool-python-parser",
-        "single-hiredis",
-        "pool-hiredis",
+        "single",
+        "pool",
     ],
 )
 async def create_valkey(request):
     """Wrapper around valkey.create_valkey."""
-    single_connection, parser_cls = request.param
+    (single_connection,) = request.param
 
     teardown_clients = []
 
@@ -76,10 +54,9 @@ async def client_factory(
         cluster_mode = VALKEY_INFO["cluster_enabled"]
         if not cluster_mode:
             single = kwargs.pop("single_connection_client", False) or single_connection
-            parser_class = kwargs.pop("parser_class", None) or parser_cls
             url_options = parse_url(url)
             url_options.update(kwargs)
-            pool = valkey.ConnectionPool(parser_class=parser_class, **url_options)
+            pool = valkey.ConnectionPool(**url_options)
             client = cls(connection_pool=pool)
         else:
             client = valkey.ValkeyCluster.from_url(url, **kwargs)