Skip to content

[ESM] Fix flaky SQS ReportBatchItemFailures test with proper visiblity timeouts #12323

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 3, 2025
Merged

[ESM] Fix flaky SQS ReportBatchItemFailures test with proper visiblity timeouts #12323

merged 2 commits into from
Mar 3, 2025

Conversation

gregfurman
Copy link
Contributor

@gregfurman gregfurman commented Mar 3, 2025 •
edited
Loading

Motivation

This PR fixes a flaky test with SQS + ESM when the ReportBatchItemFailures is set.

The duration to poll for an event was too short, meaning re-queued messages had not properly exceeded their visibility timeout -- meaning nothing was re-queued while polling was happening and that the assertion for a non-empty ReceiveMessage operation failed, resulting in a test failure.

Changes

  • Fixes test_report_batch_item_failures, ensuring the visibility timeout of the SQS queue is greater than the invocation timeout of the target lambda.

Testing

  • Test repeated against LocalStack and AWS x5 to ensure no flakiness.

@gregfurman gregfurman added aws:sqs Amazon Simple Queue Service semver: patch Non-breaking changes which can be included in patch releases aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) labels Mar 3, 2025
@gregfurman gregfurman added this to the 4.3 milestone Mar 3, 2025
@gregfurman gregfurman self-assigned this Mar 3, 2025
@gregfurman gregfurman changed the title [ESM] Fix visiblity timeouts for flaky ReportBatchItemFailures test [ESM] Fix flaky SQS ReportBatchItemFailures test with proper visiblity timeouts Mar 3, 2025
@gregfurman gregfurman mentioned this pull request Mar 3, 2025
Merged
@gregfurman gregfurman marked this pull request as ready for review March 3, 2025 12:09
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 3, 2025

LocalStack Community integration with Pro

    2 files  ±  0      2 suites  ±0   1h 28m 44s ⏱️ - 23m 14s
3 110 tests  - 994  2 890 ✅  - 882  220 💤  - 112  0 ❌ ±0 
3 112 runs   - 994  2 890 ✅  - 882  222 💤  - 112  0 ❌ ±0 

Results for commit 340a963. ± Comparison against base commit 4285fb1.

This pull request removes 994 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

joe4dev
joe4dev approved these changes Mar 3, 2025
View reviewed changes
Copy link
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

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

Great catch 👍

I like that we are more explit about many of these complex assumptions 💯

@gregfurman gregfurman merged commit abab226 into master Mar 3, 2025
33 checks passed
@gregfurman gregfurman deleted the fix/esm/flaky-batch-item-fail branch March 3, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe4dev joe4dev joe4dev approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

Assignees

@gregfurman gregfurman

Labels
aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) aws:sqs Amazon Simple Queue Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
2 participants
@gregfurman @joe4dev