Skip to content

Conversation

@carole-lavillonniere
Copy link
Contributor

@carole-lavillonniere carole-lavillonniere commented Oct 7, 2025

Motivation

While testing Event Bridge input transformation, realized boolean values were not properly rendered.

Localstack was formatting them with an upper case True and False but AWS formats them as true and false.

Changes

Added a special case for boolean values in the placeholder substitution code and added a parity test case.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

Test Results - Preflight, Unit

22 298 tests  ±0   20 555 ✅ ±0   15m 24s ⏱️ -17s
     1 suites ±0    1 743 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit b96ac19. ± Comparison against base commit 9af2464.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 7, 2025

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   58m 13s ⏱️ - 1h 0m 5s
2 653 tests  - 2 154  2 545 ✅  - 1 925  108 💤  - 229  0 ❌ ±0 
2 655 runs   - 2 154  2 545 ✅  - 1 925  110 💤  - 229  0 ❌ ±0 

Results for commit b96ac19. ± Comparison against base commit 9af2464.

This pull request removes 2156 and adds 2 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.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_create_integration_with_vpc_link
tests.aws.services.events.test_events_inputs.TestInputTransformer ‑ test_input_transformer_nested_keys_replacement["Command is <valid>!"]

♻️ This comment has been updated with latest results.

@carole-lavillonniere carole-lavillonniere force-pushed the extract-events-input-transformation branch 2 times, most recently from 91ac4a2 to f108cbf Compare October 8, 2025 09:27
@carole-lavillonniere carole-lavillonniere changed the title Fix Event Bridge input transformation Fix Event Bridge input transformation of booleans Oct 8, 2025
@carole-lavillonniere carole-lavillonniere force-pushed the extract-events-input-transformation branch from f108cbf to 1198117 Compare October 8, 2025 11:11
"call": 1.27,
"teardown": 1.23,
"total": 2.5
}
Copy link
Contributor Author

@carole-lavillonniere carole-lavillonniere Oct 8, 2025

Choose a reason for hiding this comment

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

Not sure why durations_in_seconds was added everywhere. Was the feature added after 2024-05-13?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the time tracking for test validations was added as of few months ago - thanks to @tiurin ⏲️.
The purpose of it was to use that data to track slow tests or slow services overall.

Copy link
Contributor

Choose a reason for hiding this comment

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

Context PR here: #12638

@carole-lavillonniere carole-lavillonniere added aws:events Amazon EventBridge type: bug Bug report labels Oct 8, 2025
"MessageId": "<uuid:1>",
"ReceiptHandle": "<receipt-handle:1>",
"MD5OfBody": "<m-d5-of-body:1>",
"Body": "\"Command is true!\""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Localstack used to give back "Body": "\"Command is True!\""

@carole-lavillonniere carole-lavillonniere force-pushed the extract-events-input-transformation branch from 1198117 to be0f580 Compare October 8, 2025 11:35
@carole-lavillonniere carole-lavillonniere marked this pull request as ready for review October 8, 2025 11:37
@github-actions
Copy link

github-actions bot commented Oct 8, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 20s ⏱️ -1s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit b96ac19. ± Comparison against base commit 9af2464.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 8, 2025

Test Results - Alternative Providers

380 tests   248 ✅  3m 51s ⏱️
  1 suites  132 💤
  1 files      0 ❌

Results for commit b96ac19.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 8, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   1h 25m 46s ⏱️
2 677 tests 2 572 ✅ 105 💤 0 ❌
2 683 runs  2 572 ✅ 111 💤 0 ❌

Results for commit b96ac19.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Nice and neat fix! Thanks a lot for jumping on this, adding the snapshot validated test case and re-validating other tests against AWS 🚀

I only have one minor dict regarding the docstring format, but once addressed this is good to merge!
I'll already approve the PR as this only relates to a docstring, but it would be nice to get it addressed before merging if possible 🙏

The transformed string with placeholders replaced by values from `replacements`.
"""
...
is_json_template = template.strip().startswith("{")
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: I think this is a nice change to keep the logic encapsulated, as only replace_template_placeholders needed that information. Thanks for making this nicer 👍

Comment on lines 94 to 103
"""
Replaces placeholders in an EventBridge-style InputTemplate string.
Args:
template: The template string containing placeholders like <$.foo.bar>.
replacements: A dict providing values to fill in.
Returns:
The transformed string with placeholders replaced by values from `replacements`.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: thanks for adding more detailed docstring! we do not use the Google docstring format inside our codebase, and we use Sphinx: https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html

This would look like:

    """
    Replaces placeholders in an EventBridge-style InputTemplate string.

    :param template: The template string containing placeholders like <$.foo.bar>.
    :param replacements: A dict providing values to fill in.

    :return: The transformed string with placeholders replaced by values from `replacements`.
    """

If you could update for coherence before merging, this would be awesome 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info! Addressed b96ac19

@carole-lavillonniere carole-lavillonniere merged commit 658d6fb into main Oct 8, 2025
40 checks passed
@carole-lavillonniere carole-lavillonniere deleted the extract-events-input-transformation branch October 8, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws:events Amazon EventBridge type: bug Bug report

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants