Motiva-AI / clj-sqs-extended

Client library for AWS SQS with support for large messages, up to 2GB. This is a Clojure wrapper for https://github.com/awslabs/amazon-sqs-java-extended-client-lib
MIT License
3 stars 0 forks source link

test that done-fn is doing its job when :auto-delete = false #45

Closed Quantisan closed 4 years ago

Quantisan commented 4 years ago

I noticed that there's done-fn-handle-present-when-auto-delete-false, which checks for done-fn is returned. Is there a low-level check that executing done-fn would mark the appropriate message as done? Or is that not needed you think @daisybytes ?

auto-delete both on and off states is a feature that we use regularly, so I want to make sure that's covered in the test somewhat.

ghost commented 4 years ago

I think there is no functionality of marking a message as done anymore. After the message has been read, depending on the auto-delete flag it gets deleted from the queue or not. You're right that the done-fn-handle... tests only checked for the presence of the done-fn handle so I added an additional delete-message call that throws an exception because the message was previously properly deleted by the specific parts of the code.

Would that be sufficient?

Quantisan commented 4 years ago

I think there is no functionality of marking a message as done anymore. After the message has been read, depending on the auto-delete flag it gets deleted from the queue or not. You're right that the done-fn-handle... tests only checked for the presence of the done-fn handle so I added an additional delete-message call that throws an exception because the message was previously properly deleted by the specific parts of the code.

Would that be sufficient?

@daisybytes looks good! you're right, that's what I was thinking of. we make use of auto-delete = false frequently. and if done-fn is not called, AWS would automatically re-sent the same message after visibility-timeout has passed. So checking that the message is deleted vs not deleted corresponding to auto-delete flag is exactly what I need in the test.

ghost commented 4 years ago

OK, got it. As we just agreed I'll take out the (done-fn) call from the fixture so that we can manully call the delete in the test to verify that the message was still there before.