Unskip fixed event filtering tests #11992

joe4dev · 2024-12-05T16:02:54Z

Motivation

With @bentsku's new Python-based event filtering engine, we should be able to unskip tests that previously relied on the Java engine but were skipped to avoid JPype conflicts with StepFunctions JSONata.

Changes

Unskip event filtering ESM tests

TODO

test_dynamodb_event_filter[numeric_filter] is fixed by Events: fix numeric operator string handling and provider validation for TestEventPattern #11994
Invalid filter detection for ESM should be fixed with a WIP PR by @gregfurman
test_dynamodb_event_filter[content_multiple_filters] is an ESM issue (as the skip comment says) and needs investigation -> SKIPPED for now, needs more investigation in a follow up
NICE-TO-HAVE: migrate test_sqs_event_filter to use pytest.param (Example in aws.services.lambda_.event_source_mapping.test_lambda_integration_dynamodbstreams.TestDynamoDBEventSourceMapping.test_dynamodb_event_filter) because it makes selective test execution and reporting so much easier and clearer.

bentsku · 2024-12-05T16:05:56Z

I'll create a PR shortly to fix the 2 use cases that are skipped now 👀 I didn't know about this, I would've tried to run them before! darn it 😄

bentsku · 2024-12-05T16:46:12Z

Update regarding test_dynamodb_event_filter[content_multiple_filters]: I don't think this failure is related to the Event Rule engine, but to ESM v2 directly as the skip indicated.

This is the rule:
{'eventName': ['INSERT'], 'eventSource': ['aws:dynamodb']}

And this is the payload sent by ESM to the EventRuleEngine:

{
    "eventID": "455fabb3",
    "eventName": "INSERT",
    "dynamodb": {
      "ApproximateCreationDateTime": 1733416705.0,
      "Keys": {"id": {"S": "id_value"}},
      "NewImage": {"id": {"S": "id_value"}},
      "SequenceNumber": "49658361055172123998835686422808469640999919842681159682",
      "SizeBytes": 23,
      "StreamViewType": "NEW_AND_OLD_IMAGES"
    }
}

As you can see, the eventSource field is missing from the event, which is why the test is failing. I would say that this test will fail with the Java-based event ruler as well.

bentsku · 2024-12-05T17:06:56Z

Update 2 regarding SQS:

I'm unable to reproduce test_sqs_event_filter[this is a test string] failing with the Python based event rule engine, the only failure is because of the snapshot not existing, but once regenerating the snapshot, I cannot reproduce the issue as I can clearly see this in the logs:
localstack.services.lambda_.event_source_mapping.pollers.sqs_poller : Unable to convert event body 'this is a test string' to json... Event might be dropped.

So I don't think there an issue per se with the current engine regarding ESM!

I can see in the test another failure: test_dynamodb_event_filter[numeric_filter], which I'm currently investigating 👀

EDIT: seems like test_dynamodb_event_filter[numeric_filter] is a true failure, I'm looking into it, I've added test cases in TestEventPattern.test_event_pattern and will open a fix soon 👍

EDIT2: seems like the issue stems from our handling of numeric filter with string value, will open and link the PR shortly. I believe the other issues are unrelated to the engine itself.

github-actions · 2024-12-05T17:17:53Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 1h 30m 49s ⏱️ - 21m 58s
2 948 tests - 945 2 733 ✅ - 850 215 💤 - 95 0 ❌ ±0
2 950 runs - 945 2 733 ✅ - 850 217 💤 - 95 0 ❌ ±0

Results for commit 5148fa0. ± Comparison against base commit 9f28c06.

This pull request removes 958 and adds 13 tests. Note that renamed tests count towards both.

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]
…

tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_dynamodbstreams.TestDynamoDBEventSourceMapping ‑ test_dynamodb_event_filter[exists_false_filter]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_dynamodbstreams.TestDynamoDBEventSourceMapping ‑ test_dynamodb_event_filter[numeric_filter]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[and]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[exists]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[numeric-bigger]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[numeric-range]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[numeric-smaller]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[or]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[plain-string-filter]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[plain-string-matching]
…

This pull request removes 97 skipped tests and adds 2 skipped tests. Note that renamed tests count towards both.

tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_api_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_invalid_desiredstate
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_double_create_with_client_token
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_lifecycle
…

tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[plain-string-filter]
tests.aws.services.lambda_.event_source_mapping.test_lambda_integration_sqs.TestSQSEventSourceMapping ‑ test_sqs_event_filter[plain-string-matching]

♻️ This comment has been updated with latest results.

bentsku · 2024-12-05T18:04:01Z

Update on all issues:

test_dynamodb_event_filter[numeric_filter]: I've opened Events: fix numeric operator string handling and provider validation for TestEventPattern #11994 to fix it, which is now confirmed to work.
test_dynamodb_event_filter[content_multiple_filters] is not related to the EventRuleEngine I believe, but to ESM not populating a field before passing it to the engine
I was unable to make test_sqs_event_filter[this is a test string] fail, @joe4dev feel free to send it my way if you still have issues

All the other test failures seems to be related to the validation of the rule itself.

Thanks again @joe4dev for raising this!

joe4dev · 2024-12-05T19:20:29Z

Update on all issues:

* `test_dynamodb_event_filter[numeric_filter]`: I've opened [Events: fix `numeric` operator string handling and provider validation for TestEventPattern #11994](https://github.com/localstack/localstack/pull/11994) to fix it, which is now confirmed to work.

* `test_dynamodb_event_filter[content_multiple_filters]` is not related to the EventRuleEngine I believe, but to ESM not populating a field before passing it to the engine

* I was unable to make `test_sqs_event_filter[this is a test string]` fail, @joe4dev feel free to send it my way if you still have issues

All the other test failures seems to be related to the validation of the rule itself.

Thanks again @joe4dev for raising this!

Wow, amazing quick updates @bentsku 🙇 I fully agree with your conclusion:

test_sqs_event_filter[this is a test string] was a false alarm (confirmed by CI)
test_dynamodb_event_filter[content_multiple_filters] is an ESM issue (as the skip comment says)
test_dynamodb_event_filter[numeric_filter] was the only problematic one. Thanks for the quick fix 🥇

dfangl

More test coverage is always good :D

joe4dev added the semver: patch Non-breaking changes which can be included in patch releases label Dec 5, 2024

joe4dev self-assigned this Dec 5, 2024

bentsku mentioned this pull request Dec 5, 2024

Events: fix numeric operator string handling and provider validation for TestEventPattern #11994

Merged

joe4dev force-pushed the unskip-fixed-event-filtering-tests branch 3 times, most recently from 1f5f2cc to 0dfc89c Compare December 11, 2024 15:48

joe4dev changed the base branch from master to fix/esm/check-filter-criteria December 11, 2024 15:51

Base automatically changed from fix/esm/check-filter-criteria to master December 11, 2024 16:01

joe4dev force-pushed the unskip-fixed-event-filtering-tests branch from 0dfc89c to 766deaf Compare December 11, 2024 16:05

joe4dev marked this pull request as ready for review December 13, 2024 08:50

joe4dev requested review from dominikschubert, dfangl and gregfurman as code owners December 13, 2024 08:50

joe4dev added 6 commits December 13, 2024 14:26

Unskip event filtering tests passing with new Python engine

f8e18d3

Uncomment more event filtering tests

9f88f7f

Update snapshots for skipped test scenarios

b12aa8f

Parametrize and extend SQS ESM filtering test

8b8a7e5

Skip failing content_multiple_filters test case

0069784

Fix skip marker

5148fa0

joe4dev force-pushed the unskip-fixed-event-filtering-tests branch from 59c53d8 to 5148fa0 Compare December 13, 2024 13:26

dfangl approved these changes Dec 13, 2024

View reviewed changes

joe4dev merged commit 42335ca into master Dec 14, 2024
32 checks passed

joe4dev deleted the unskip-fixed-event-filtering-tests branch December 14, 2024 11:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Unskip fixed event filtering tests #11992

Unskip fixed event filtering tests #11992

Uh oh!

joe4dev commented Dec 5, 2024 •

edited

Loading

Uh oh!

bentsku commented Dec 5, 2024 •

edited

Loading

Uh oh!

bentsku commented Dec 5, 2024 •

edited

Loading

Uh oh!

bentsku commented Dec 5, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Dec 5, 2024 •

edited

Loading

Uh oh!

bentsku commented Dec 5, 2024 •

edited

Loading

Uh oh!

joe4dev commented Dec 5, 2024

Uh oh!

dfangl left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Unskip fixed event filtering tests #11992

Unskip fixed event filtering tests #11992

Uh oh!

Conversation

joe4dev commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

TODO

Uh oh!

bentsku commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bentsku commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bentsku commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

bentsku commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joe4dev commented Dec 5, 2024

Uh oh!

dfangl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joe4dev commented Dec 5, 2024 •

edited

Loading

bentsku commented Dec 5, 2024 •

edited

Loading

bentsku commented Dec 5, 2024 •

edited

Loading

bentsku commented Dec 5, 2024 •

edited

Loading

github-actions bot commented Dec 5, 2024 •

edited

Loading

bentsku commented Dec 5, 2024 •

edited

Loading