Skip to content

Conversation

@jh1231223
Copy link
Contributor

@jh1231223 jh1231223 commented Oct 13, 2025

Fixes #4364 and fixes requested in #4862

This PR now includes the changes from #4886 and stabilizes the following tests in the same file:
• test_pubsub_combined_exact_and_pattern_one_client
• test_pubsub_combined_exact_and_pattern_multiple_clients
• test_pubsub_combined_exact_pattern_and_sharded_one_client
• test_pubsub_combined_exact_pattern_and_sharded_multi_client

Approach: use unique channel names per test and ensure subscription setup completes before publishing. Verified with repeated runs; no intermittent failures observed.

Context

test_pubsub_combined_exact_and_pattern_one_client was occasionally failing with off-by-one mismatches in message counts.
The root cause was random collisions between exact and pattern channel names generated separately in two comprehensions.

What I changed

  • Switched both comprehensions to for i in range(NUM_CHANNELS)
  • Included the loop index in channel names to ensure deterministic uniqueness

Why

This prevents accidental overlap between channel sets, stabilizing message counts across exact and pattern subscriptions.

Validation

Tested locally with repeated runs:

python -m pytest -c /dev/null -q \
  tests/async_tests/test_pubsub.py \
  -k "test_pubsub_combined_exact_and_pattern_one_client" \
  --count=200 -s --maxfail=1

@jh1231223 jh1231223 requested a review from a team as a code owner October 13, 2025 09:11
@jh1231223 jh1231223 force-pushed the fix-pubsub-deterministic-channels branch from 7e82b35 to f3a541f Compare October 13, 2025 09:14
…s in combined exact+pattern test (valkey-io#4364)

Signed-off-by: Jessica Hsiao <136421644+jh1231223@users.noreply.github.com>
@jh1231223 jh1231223 force-pushed the fix-pubsub-deterministic-channels branch from f3a541f to 3f6a704 Compare October 13, 2025 09:44
Signed-off-by: Jessica Hsiao <j.hsiao1998@gmail.com>
@xShinnRyuu
Copy link
Collaborator

Additional ask in ticket: #4862 (comment)

…ub_combined_exact_pattern_and_sharded_multi_client

Signed-off-by: Jessica Hsiao <j.hsiao1998@gmail.com>
…attern_and_sharded_one_client' into fix-pubsub-deterministic-channels

Merge valkey-io#4886 into valkey-io#4880 to combine flaky Pub/Sub test fixes

Includes fixes from fix-test_pubsub_combined_exact_pattern_and_sharded_one_client
into fix-pubsub-deterministic-channels as requested in issue valkey-io#4862.
Stabilizes all related Pub/Sub tests with unique channel names and proper sync.#
@xShinnRyuu
Copy link
Collaborator

Thanks for merging them all into one PR. It helps to keep readability and for potential troubleshooting down the line. PR looks good, but there are some linting errors from CD/CI you need to address.

@jh1231223 Please fix python lint errors

Signed-off-by: Jessica Hsiao <j.hsiao1998@gmail.com>
@jh1231223
Copy link
Contributor Author

@xShinnRyuu I reformatted the file with Black. Lint errors should be clean now. Thanks!

Copy link
Collaborator

@xShinnRyuu xShinnRyuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@jamesx-improving jamesx-improving left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for your contribution!

@jamesx-improving jamesx-improving merged commit 6ac8b6a into valkey-io:main Oct 30, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python][Flaky Test] test_pubsub_combined_exact_and_pattern_one_client[trio-MethodTesting.Callback-False]

3 participants