Skip to content

CFNV2: improve parity with describe operations #12980

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 13 commits into from
Aug 11, 2025
Merged

CFNV2: improve parity with describe operations #12980

merged 13 commits into from
Aug 11, 2025

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Aug 8, 2025 •
edited
Loading

Motivation

We have a few tests that depend on describing stacks or change sets. These tests verify the parity of the stacks and change sets.

Changes

  • Better handling of stack outputs, using the API type
  • Don't extract exports in the executor, instead extract in the provider
  • Include the stack description and extract from the template but only after creation is complete
  • Remove describe_details from the Stack and put it in the provider where it belongs
  • Don't set the change set id until the change set has been executed
  • Generally only set stack details once the change set has been executed
  • Add NotificationARNs property (and remove the snapshot skips relating to it)
  • Only add the Parameters field when describing a change set if parameters are present
  • Extract _describe_stack method for use in list_stacks and describe_stacks

@simonrw simonrw added this to the 4.8 milestone Aug 8, 2025
@simonrw simonrw added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases review: merge when ready Signals to the reviewer that a PR can be merged if accepted labels Aug 8, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2025 •
edited
Loading

Test Results - Preflight, Unit

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

Results for commit a4a7199. ± Comparison against base commit c1d19e2.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2025 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   21m 32s ⏱️ - 1h 24m 57s
554 tests  - 4 071  329 ✅  - 3 857  225 💤  - 214  0 ❌ ±0 
556 runs   - 4 071  329 ✅  - 3 857  227 💤  - 214  0 ❌ ±0 

Results for commit a4a7199. ± Comparison against base commit c1d19e2.

This pull request removes 4072 and adds 1 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.cloudformation.api.test_stacks.TestStacksApi ‑ test_stack_description_lifecycle
This pull request removes 215 skipped tests and adds 1 skipped test. 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.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_get_api_case_insensitive
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_update_rest_api_concatenation_of_errors
tests.aws.services.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_integration_response_invalid_responsetemplates
tests.aws.services.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_put_integration_request_parameter_bool_type
tests.aws.services.apigateway.test_apigateway_basic.TestAPIGateway ‑ test_api_gateway_authorizer_crud
…

tests.aws.services.cloudformation.api.test_stacks.TestStacksApi ‑ test_stack_description_lifecycle

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

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

Results for commit a4a7199. ± Comparison against base commit c1d19e2.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 8, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

  5 files  ±    0    5 suites  ±0   33m 29s ⏱️ - 1h 47m 33s
578 tests  - 4 406  354 ✅  - 4 039  224 💤  - 367  0 ❌ ±0 
584 runs   - 4 406  354 ✅  - 4 039  230 💤  - 367  0 ❌ ±0 

Results for commit a4a7199. ± Comparison against base commit c1d19e2.

This pull request removes 4407 and adds 1 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.cloudformation.api.test_stacks.TestStacksApi ‑ test_stack_description_lifecycle
This pull request removes 368 skipped tests and adds 1 skipped test. 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.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_get_api_case_insensitive
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiRestApi ‑ test_update_rest_api_concatenation_of_errors
tests.aws.services.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_integration_response_invalid_responsetemplates
tests.aws.services.apigateway.test_apigateway_api.TestApigatewayIntegration ‑ test_put_integration_request_parameter_bool_type
tests.aws.services.apigateway.test_apigateway_basic.TestAPIGateway ‑ test_api_gateway_authorizer_crud
…

tests.aws.services.cloudformation.api.test_stacks.TestStacksApi ‑ test_stack_description_lifecycle

♻️ This comment has been updated with latest results.

@simonrw simonrw marked this pull request as ready for review August 8, 2025 22:08
@simonrw simonrw mentioned this pull request Aug 9, 2025
Merged
@coveralls
Copy link

Coverage Status

coverage: 66.364% (-20.5%) from 86.837%
when pulling a4a7199 on cfn/v2/describing
into c1d19e2 on main.

pinzon
pinzon approved these changes Aug 11, 2025
View reviewed changes
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 👍

@simonrw simonrw merged commit d6ee93e into main Aug 11, 2025
40 checks passed
@simonrw simonrw deleted the cfn/v2/describing branch August 11, 2025 18:51
@simonrw simonrw mentioned this pull request Aug 11, 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
review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Milestone
4.8
Development

Successfully merging this pull request may close these issues.
3 participants
@simonrw @coveralls @pinzon