test: fix flaky test_request_queue_unlock_requests#786
Open
vdusek wants to merge 1 commit into
Open
Conversation
vdusek
added
adhoc
github-actions
Bot
added
the
tested
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #786 +/- ##
==========================================
- Coverage 97.81% 97.73% -0.09%
==========================================
Files 48 48
Lines 4860 4860
==========================================
- Hits 4754 4750 -4
- Misses 106 110 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vdusek
force-pushed
the
test/fix-flaky-request-queue-unlock
branch
2 times, most recently
from
a1bb13f to
2ae6dfa
Compare
vdusek
force-pushed
the
test/fix-flaky-request-queue-unlock
branch
from
2ae6dfa to
85b27a5
Compare
janbuchar
approved these changes
May 11, 2026
Comment on lines
408
to
+426
| # Add multiple requests | ||
| for i in range(5): | ||
| await maybe_await( | ||
| rq_client.add_request({'url': f'https://example.com/lock-{i}', 'unique_key': f'lock-{i}'}) | ||
| ) | ||
|
|
||
| # Poll until requests are available for locking (eventual consistency) | ||
| lock_response: LockedRequestQueueHead | None = None | ||
| # Poll (without side effects) until all requests are visible. Using `list_and_lock_head` for polling | ||
| # would lock items across iterations, leading to an ambiguous count of actually-locked requests. | ||
| head_response: RequestQueueHead | None = None | ||
| for _ in range(5): | ||
| await maybe_sleep(1, is_async=is_async) | ||
| result = await maybe_await(rq_client.list_and_lock_head(limit=3, lock_duration=timedelta(seconds=60))) | ||
| assert isinstance(result, LockedRequestQueueHead) | ||
| lock_response = result | ||
| if len(lock_response.items) == 3: | ||
| result = await maybe_await(rq_client.list_head(limit=5)) | ||
| assert isinstance(result, RequestQueueHead) | ||
| head_response = result | ||
| if len(head_response.items) == 5: | ||
| break | ||
|
|
||
| assert lock_response is not None | ||
| assert head_response is not None | ||
| assert len(head_response.items) == 5 |
Contributor
There was a problem hiding this comment.
This could be a helper function instead (ensure_queue_is_populated?), calling list_head in a test focused on list_and_lock_head gives off bad vibes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix this flaky CI run of
test_request_queue_unlock_requests[sync], which failed withassert 2 == 3onunlock_response.unlocked_count.Root cause: the polling loop called
list_and_lock_headrepeatedly, which has the side effect of locking items. Across multiple iterations (due to eventual consistency onadd_request) the total set of actually-locked requests no longer matched the last response'sitemscount, sounlock_requestscould report a different count than the test expected.Fix: separate the two concerns — poll the read-only
list_headuntil all 5 added requests are visible, then calllist_and_lock_head(limit=3)exactly once. This removes the cross-iteration locking ambiguity.Applied the same treatment to
test_request_queue_list_and_lock_head, which shared the same racy polling pattern.