Skip to content

CFNV2: fix select in mapping test #12993

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
simonrw merged 1 commit into from
Aug 12, 2025
Merged

CFNV2: fix select in mapping test #12993

simonrw merged 1 commit into from
Aug 12, 2025

Conversation

@simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 11, 2025

Motivation

I merged a PR (#12980) without rebasing on top of #12955, and so introduced an error that was not caught in each individual PR. My PR added stricter validations of types when looking up values in maps, and the test involved looking up values in maps. The return type for the lookup was an array which was not included in my allowlist of valid types. As such a ValidationError is thrown when it should not be.

Changes

Fix the test to include NodeArray and NodeObject types as well as TerminalValue. Note: the secondary level key is visited, so if it is itself a nested intrinsic function (if that's even possible in CFn) then the value is subsequently resolved.

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Aug 11, 2025
@github-actions
Copy link

github-actions bot commented Aug 11, 2025

Test Results - Preflight, Unit

22 063 tests  ±0   20 329 ✅ ±0   6m 11s ⏱️ -34s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 3ad8771. ± Comparison against base commit 4258f76.

@github-actions
Copy link

github-actions bot commented Aug 11, 2025

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   21m 30s ⏱️ - 1h 21m 23s
555 tests  - 4 073  330 ✅  - 3 858  225 💤  - 215  0 ❌ ±0 
557 runs   - 4 073  330 ✅  - 3 858  227 💤  - 215  0 ❌ ±0 

Results for commit 3ad8771. ± Comparison against base commit 4258f76.

This pull request removes 4073 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]
…

@github-actions
Copy link

github-actions bot commented Aug 11, 2025

Test Results (amd64) - Acceptance

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

Results for commit 3ad8771. ± Comparison against base commit 4258f76.

@github-actions
Copy link

github-actions bot commented Aug 11, 2025

Test Results (amd64) - Integration, Bootstrap

  5 files    5 suites   34m 13s ⏱️
579 tests 355 ✅ 224 💤 0 ❌
585 runs  355 ✅ 230 💤 0 ❌

Results for commit 3ad8771.

@simonrw simonrw marked this pull request as ready for review August 12, 2025 00:43
@simonrw simonrw merged commit a74c9f9 into main Aug 12, 2025
43 checks passed
@simonrw simonrw deleted the cfn/v2/fix-validations branch August 12, 2025 00:44
@simonrw simonrw mentioned this pull request Sep 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pinzon pinzon pinzon approved these changes

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

Assignees

No one assigned

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@simonrw @pinzon