valkey-io / valkey-py

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

Flaky test_geo* #39

Closed aiven-sal closed 3 weeks ago

aiven-sal commented 1 month ago

Some tests that match test_geo* in tests/test_commands.py and tests/test_asyncio/test_commands.py are not reliable. We never observed them failing in CI or locally, but I received reports that they failed in some occasions.

These tests expect exact matches between floats e.g.

https://github.com/valkey-io/valkey-py/blob/6c95904b07bee87f6a6fd736a8557f166e48de53/tests/test_commands.py#L3544-L3563

And this doesn't always work.

According to the reports I received, the values are usually off by ~1e-15

Floats should be compared using math.isclose. The default tolerance should be enough.

aiven-sal commented 1 month ago

These are the tests that have been reported as failing:

FAILED tests/test_commands.py::TestValkeyCommands::test_geopos - assert [(2.1...
FAILED tests/test_commands.py::TestValkeyCommands::test_geosearch_member - As...
FAILED tests/test_commands.py::TestValkeyCommands::test_geosearch_with - Asse...
FAILED tests/test_commands.py::TestValkeyCommands::test_geosearchstore_dist
FAILED tests/test_commands.py::TestValkeyCommands::test_georadius_with - Asse...
FAILED tests/test_commands.py::TestValkeyCommands::test_georadius_store_dist
FAILED tests/test_commands.py::TestValkeyCommands::test_georadiusmember - Ass...
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_geopos[single]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_geopos[pool]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadius_with[single]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadius_with[pool]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadius_store_dist[single]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadius_store_dist[pool]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadiusmember[single]
FAILED tests/test_asyncio/test_commands.py::TestValkeyCommands::test_georadiusmember[pool]

here is the full build log: https://koji.fedoraproject.org/koji/taskinfo?taskID=120194663

The error happens on s390x, it may be impossible to reproduce on other architectures.

kjaymiller commented 1 month ago

would testing on a docker image as mentioned in the link below work?

https://docs.gitlab.com/omnibus/development/s390x.html#vm-provisioning

I should have a PR in today or tomorrow and can take a little more time to try and run on this if you believe it will effectively test.

[UPDATE]

I seem to be having the same test failures on my mac (m2 pro). So perhaps we don't need an s390x to verify the tests now pass