Repository navigation
test(cluster): skip the MSETEX multi-shard test below 8.3.224 - #4601
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
The only failing check is |
| assert await pipe.execute() == [0] | ||
| assert keyslot.call_count == len(keys) | ||
|
|
||
| @skip_if_server_version_lt("8.3.224") |
There was a problem hiding this comment.
Hey @BlueX888, thank you for your contribution!
Please gate the tests with 8.4 - we already have this officially released. Those pre-release versions are used only while we test the milestone build before the official releases. After the GA is out, we can safely use the release version, which in this case is 8.4.
petyaslavova
added
maintenance
|
Done in 8f53b33: both gates now use the 8.4 release instead of 8.3.224. Verified against a Redis 8.2.11 server — both tests skip with |
petyaslavova
left a comment
petyaslavova
left a comment
There was a problem hiding this comment.
Great, thanks @BlueX888. LGTM.
Description of change
master CI has been failing since 393ec3c (#4569) added
test_keys_that_cannot_be_located_are_not_splitfor both cluster clients. The test sendsMSETEX, which the push-only CI cells (Redis 8.0.6 and 8.2.11) do not have, soCommandsParserraisesRedisError: MSETEX command doesn't exist in Redis commandsbefore the test reaches the split logic. The pull-request matrix omits those two server versions, which is why #4569's own run was green.MSETEXis a new command. Gate both tests on the official 8.4 release (@skip_if_server_version_lt("8.4")) intests/test_cluster.pyandtests/test_asyncio/test_cluster.py; per review, pre-release numbers such as 8.3.224 are only used for milestone builds. The test still runs on the 8.4+ cells (8.4.8, 8.6.8, 8.8.4, current 8.10.3, 8.12.0) and skips only where the command does not exist. The class-level>= 8.0.0gate skips the 7.2.14 pull-request cell.Verified with a Redis 8.2.11 server (the failing CI version) and Redis 7.4.11: on both, the sync test skips (1 skipped) and the async test skips (2 skipped) with
Redis version required >= 8.4; on 8.2.11 the class-level>= 8.0.0gate is false, so the new gate is what skips them.ruff checkandruff format --checkpass. The main CI failure (run 42544303863 on 4fe8a8c) shows only these two tests failing.Pull Request check-list
Note
Low Risk
Test-only version gating with no production or library behavior changes.
Overview
Both cluster
test_keys_that_cannot_be_located_are_not_splitcases (sync intests/test_cluster.py, asyncio intests/test_asyncio/test_cluster.py) now use@skip_if_server_version_lt("8.4")so they only run whenMSETEXexists on the server.CI on Redis 8.0.6 and 8.2.11 was failing because
CommandsParserrejectsMSETEXbefore the test exercises multi-shard split behavior; those matrix cells now skip instead of erroring. The tests still run on 8.4+ CI images.Reviewed by Cursor Bugbot for commit c3a2fa8. Bugbot is set up for automated code reviews on this repo. Configure here.