Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

630: Only nack messages upon cancellation of a consumer subscription … #634

Conversation

lfse-slafleur
Copy link
Contributor

…that belong to that consumer subscription.

@lfse-slafleur
Copy link
Contributor Author

Closes #630

@lfse-slafleur
Copy link
Contributor Author

lfse-slafleur commented Jun 25, 2024

Took awhile due to illness, vacation and other priorities. It also wasn't easy to find a proper, simple regression test, but I think I found one! Following TDD: The test was giving the expected exception before fixing the code. After fixing the code, the test succeeded.

@@ -1587,6 +1587,141 @@ async def test_heartbeat_disabling(
async with connection:
assert heartbeat == 0

async def test_non_acked_messages_are_redelivered_to_queue(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test to see if messages are properly nack'ed and I didn't break anything.

# Cleanup, delete the queue
await queue.delete()

async def test_regression_only_messages_cancelled_subscription_are_nacked(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regression test which failed before fixing the code. Documentation is inline to show why the test is constructed the way it is.

@coveralls
Copy link

Coverage Status

coverage: 88.316% (+0.2%) from 88.125%
when pulling c7226b1 on lfse-slafleur:630-messages-are-nacked-multiple-times-around-queue-deletion
into b610628 on mosquito:master.

@mosquito mosquito merged commit 43f7b8a into mosquito:master Jul 1, 2024
9 checks passed
@mosquito
Copy link
Owner

mosquito commented Jul 1, 2024

@lfse-slafleur released as 9.4.2

@lfse-slafleur lfse-slafleur deleted the 630-messages-are-nacked-multiple-times-around-queue-deletion branch July 1, 2024 14:00
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.

3 participants