search

valkey-io / valkey-py

Valkey Python client based on a fork of redis-py
MIT License
21 stars 2 forks source link

missing pytest.mark.skipif on TestSSLConnectionURLParsing #26

Open kjaymiller opened 4 days ago

kjaymiller commented 4 days ago

Version: 5.1.0b7

Platform: Python 3.12 / MacOS Sonoma in M1 Arm Device

Description:

There is an inconsistent skip on testing if SSL isn't installed between the connection pool in the sync and async tests.

These tests are otherwise identical (pointing to my suggested issue #25)

sync: https://github.com/valkey-io/valkey-py/blob/d116aa6efa03877a0c37598139a556554ad47c13/tests/test_connection_pool.py#L456C1-L494C1

async: https://github.com/valkey-io/valkey-py/blob/d116aa6efa03877a0c37598139a556554ad47c13/tests/test_asyncio/test_connection_pool.py#L543-L572

Unless there is a reason the async client should not skip the tests (or a particular reason the synchronous one should) is it not better to make the tests test the same thing the same way (or implement #25 and condense that logic into a module that both versions use which would eliminate the need for testing in both areas.

aiven-sal commented 4 days ago

Yes, the skipif should be added to the async test as well, but I'd keep the 2 tests separate. The tests should not know/care if valkey and valkey.asyncio share some implementation detail or not.