Skip to content

CFNV2: fix remaining transform tests #13010

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.

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

Merged
merged 2 commits into from
Aug 19, 2025
Merged

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 14, 2025

Motivation

With #12966 a couple of transform tests were left unskipped

Changes

  • Handle Fn::Transform in the resource position
  • Mark the stack as modified if a transform is present (see test_update_stack_with_same_template_withoutchange_transformation!)
  • Skip two transform tests
  • Remove two skipped tests that have no need to exist any more

@simonrw simonrw added this to the 4.8 milestone Aug 14, 2025
@simonrw simonrw added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Aug 14, 2025
Copy link

github-actions bot commented Aug 14, 2025

Test Results - Preflight, Unit

22 144 tests  ±0   20 407 ✅ ±0   6m 34s ⏱️ +12s
     1 suites ±0    1 737 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit ab6f2d7. ± Comparison against base commit 3f463b5.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 14, 2025

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   21m 17s ⏱️ - 1h 22m 43s
555 tests  - 4 078  330 ✅  - 3 863  225 💤  - 215  0 ❌ ±0 
557 runs   - 4 078  330 ✅  - 3 863  227 💤  - 215  0 ❌ ±0 

Results for commit ab6f2d7. ± Comparison against base commit 3f463b5.

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

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 14, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 18s ⏱️ +11s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit ab6f2d7. ± Comparison against base commit 3f463b5.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Aug 14, 2025

Test Results (amd64) - Integration, Bootstrap

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

Results for commit ab6f2d7.

♻️ This comment has been updated with latest results.

Base automatically changed from cp/cfn/v2/transform to main August 14, 2025 22:49
@simonrw simonrw force-pushed the cfn/v2/fix-remaining-transform-tests branch from fc021c5 to e6d889f Compare August 15, 2025 09:25
@simonrw simonrw marked this pull request as ready for review August 15, 2025 14:04
@simonrw simonrw added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Aug 15, 2025
@simonrw simonrw force-pushed the cfn/v2/fix-remaining-transform-tests branch 2 times, most recently from 875931e to a69f74c Compare August 19, 2025 16:24
Copy link
Member

@pinzon pinzon left a comment

Choose a reason for hiding this comment

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

LGTM. just one question before merging.

@@ -152,7 +155,7 @@ def to_change_action(self) -> ChangeAction:

class ChangeSetEntity(abc.ABC):
scope: Final[Scope]
change_type: Final[ChangeType]
change_type: ChangeType
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes because we overwrite it in the provider and the type checker complains if we leave the final in

@simonrw simonrw force-pushed the cfn/v2/fix-remaining-transform-tests branch from a7f4742 to 976fad7 Compare August 19, 2025 18:39
@simonrw simonrw removed the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Aug 19, 2025
@simonrw simonrw force-pushed the cfn/v2/fix-remaining-transform-tests branch from 976fad7 to ab6f2d7 Compare August 19, 2025 18:40
@simonrw simonrw force-pushed the cfn/v2/fix-remaining-transform-tests branch from ab6f2d7 to f9e466d Compare August 19, 2025 20:14
@simonrw simonrw merged commit b990f66 into main Aug 19, 2025
13 checks passed
simonrw added a commit that referenced this pull request Aug 19, 2025
@simonrw simonrw deleted the cfn/v2/fix-remaining-transform-tests branch August 19, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants