From 8e8309e96c9b3565571db0ee755e05912c126a7b Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 29 Oct 2025 12:52:29 -0400 Subject: [PATCH 01/13] Refactor model-validation tests to specify the target on the last line of the error string. --- tests/functional/botocore/test_h2_required.py | 11 ++- .../botocore/test_paginator_config.py | 93 +++++++++++-------- .../botocore/test_supported_protocols.py | 10 +- tests/functional/test_no_event_streams.py | 10 +- tests/functional/test_shadowing.py | 8 +- 5 files changed, 85 insertions(+), 47 deletions(-) diff --git a/tests/functional/botocore/test_h2_required.py b/tests/functional/botocore/test_h2_required.py index 97aa73b58bd5..72f446c2e774 100644 --- a/tests/functional/botocore/test_h2_required.py +++ b/tests/functional/botocore/test_h2_required.py @@ -55,7 +55,10 @@ def _all_test_cases(): @pytest.mark.parametrize("h2_service", H2_SERVICES) def test_all_uses_of_h2_are_known(h2_service): # Validates that a service that requires HTTP 2 for all operations is known - message = f'Found unknown HTTP 2 service: {h2_service}' + message = ( + f'Found unknown HTTP 2 service: {h2_service}\n' + f'Target={h2_service}' + ) assert _KNOWN_SERVICES.get(h2_service) is _H2_REQUIRED, message @@ -64,5 +67,9 @@ def test_all_uses_of_h2_are_known(h2_service): def test_all_h2_operations_are_known(h2_service, operation): # Validates that an operation that requires HTTP 2 is known known_operations = _KNOWN_SERVICES.get(h2_service, []) - message = f'Found unknown HTTP 2 operation: {h2_service}.{operation}' + message = ( + f'Found unknown HTTP 2 operation: {h2_service}.{operation}\n' + f"Target={h2_service}.{operation}" + ) + assert operation in known_operations, message diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 45af7578ec7a..252d6602a280 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -149,51 +149,61 @@ def _pagination_configs(): @pytest.mark.parametrize( "operation_name, page_config, service_model", _pagination_configs() ) -def test_lint_pagination_configs(operation_name, page_config, service_model): - _validate_known_pagination_keys(page_config) - _validate_result_key_exists(page_config) - _validate_referenced_operation_exists(operation_name, service_model) - _validate_operation_has_output(operation_name, service_model) - _validate_input_keys_match(operation_name, page_config, service_model) - _validate_output_keys_match(operation_name, page_config, service_model) - _validate_new_numeric_keys(operation_name, page_config, service_model) - - -def _validate_known_pagination_keys(page_config): +def test_known_pagination_keys(operation_name, page_config, service_model): for key in page_config: if key not in KNOWN_PAGE_KEYS: raise AssertionError( - f"Unknown key '{key}' in pagination config: {page_config}" + f"Unknown key '{key}' in pagination config: {page_config}\n" + f"Target={operation_name}.{key}" ) -def _validate_result_key_exists(page_config): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_result_key_exists(operation_name, page_config, service_model): if 'result_key' not in page_config: raise AssertionError( "Required key 'result_key' is missing " - f"from pagination config: {page_config}" + f"from pagination config: {page_config}\n" + f"Code=ResultKeyMissing\nTarget={operation_name}.result_key" ) -def _validate_referenced_operation_exists(operation_name, service_model): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_referenced_operation_exists(operation_name, page_config, service_model): if operation_name not in service_model.operation_names: raise AssertionError( "Pagination config refers to operation that " - f"does not exist: {operation_name}" + f"does not exist: {operation_name}\n" + f"Target={operation_name}" ) -def _validate_operation_has_output(operation_name, service_model): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_operation_has_output(operation_name, page_config, service_model): op_model = service_model.operation_model(operation_name) output = op_model.output_shape if output is None or not output.members: raise AssertionError( "Pagination config refers to operation " - f"that does not have any output: {operation_name}" + f"that does not have any output: {operation_name}\n" + f"Target={operation_name}" ) -def _validate_input_keys_match(operation_name, page_config, service_model): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_input_keys_match(operation_name, page_config, service_model): input_tokens = page_config['input_token'] if not isinstance(input_tokens, list): input_tokens = [input_tokens] @@ -203,24 +213,27 @@ def _validate_input_keys_match(operation_name, page_config, service_model): for token in input_tokens: if token not in valid_input_names: raise AssertionError( - f"input_token '{token}' refers to a non existent " - f"input member for operation: {operation_name}" + f"input_token refers to a non existent " + f"input member for operation.\n" + f"Target={operation_name}.{token}" ) if 'limit_key' in page_config: limit_key = page_config['limit_key'] if limit_key not in valid_input_names: + valid_keys = ', '.join(list(valid_input_names)) raise AssertionError( - "limit_key '{}' refers to a non existent " - "input member for operation: {}, valid keys: " - "{}".format( - limit_key, - operation_name, - ', '.join(list(valid_input_names)), - ) + f"limit_key '{limit_key}' refers to a non existent " + f"input member for operation: {operation_name}, valid keys: " + f"{valid_keys}.\n" + f"Target={operation_name}.{limit_key}" ) -def _validate_output_keys_match(operation_name, page_config, service_model): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_output_keys_match(operation_name, page_config, service_model): # NOTE: The original version of this function from translate.py had logic # to ensure that the entire set of output_members was accounted for in the # union of 'result_key', 'output_token', 'more_results', and @@ -237,7 +250,8 @@ def _validate_output_keys_match(operation_name, page_config, service_model): if output_key not in output_members: raise AssertionError( f"Pagination key '{key_name}' refers to an output " - f"member that does not exist: {output_key}" + f"member that does not exist: {output_key}\n" + f"Target={operation_name}.{output_key}" ) output_members.remove(output_key) @@ -253,16 +267,18 @@ def _validate_output_keys_match(operation_name, page_config, service_model): f.write(f"'{key}',\n") raise AssertionError( "There are member names in the output shape of " - "{} that are not accounted for in the pagination " - "config for service {}: {}".format( - operation_name, - service_model.service_name, - ', '.join(output_members), - ) + f"{operation_name} that are not accounted for in the pagination " + f"config for service {service_model.service_name}: " + f"[{', '.join(output_members)}]\n" + f"Target={operation_name}[{', '.join(output_members)}]" ) -def _validate_new_numeric_keys(operation_name, page_config, service_model): +@pytest.mark.validates_models +@pytest.mark.parametrize( + "operation_name, page_config, service_model", _pagination_configs() +) +def test_new_numeric_keys(operation_name, page_config, service_model): output_shape = service_model.operation_model(operation_name).output_shape for key in _get_list_value(page_config, 'result_key'): current_shape = output_shape @@ -282,7 +298,8 @@ def _validate_new_numeric_keys(operation_name, page_config, service_model): f'{service_model.service_name} that is configured to sum ' 'integer outputs across pages. Verify that this behavior is ' 'correct before allow-listing, since whether or not it is ' - 'appropriate to sum depends on the subject matter.' + 'appropriate to sum depends on the subject matter.\n' + f'Target={service_model.service_name}.{operation_name}' ) diff --git a/tests/functional/botocore/test_supported_protocols.py b/tests/functional/botocore/test_supported_protocols.py index f14bb9a2ec97..076fe1c0d46c 100644 --- a/tests/functional/botocore/test_supported_protocols.py +++ b/tests/functional/botocore/test_supported_protocols.py @@ -53,7 +53,10 @@ def _single_protocol_test_cases(): def test_services_with_protocols_trait_have_supported_protocol( service_name, supported_protocols ): - message = f"No protocols supported for service {service_name}" + message = ( + f"No protocols supported for service {service_name}" + f"\nTarget={service_name}" + ) assert any( protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS for protocol in supported_protocols @@ -68,5 +71,8 @@ def test_services_with_protocols_trait_have_supported_protocol( def test_services_without_protocols_trait_have_supported_protocol( service_name, supported_protocol ): - message = f"Service protocol not supported for {service_name}" + message = ( + f"Service protocol not supported for {service_name}" + f"\nTarget={service_name}" + ) assert supported_protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS, message diff --git a/tests/functional/test_no_event_streams.py b/tests/functional/test_no_event_streams.py index fd3fbcb457cc..68330809aa28 100644 --- a/tests/functional/test_no_event_streams.py +++ b/tests/functional/test_no_event_streams.py @@ -24,6 +24,7 @@ def test_no_event_stream_unless_allowed(): driver = create_clidriver() help_command = driver.create_help_command() errors = [] + targets = [] for command_name, command_obj in help_command.command_table.items(): sub_help = command_obj.create_help_command() if hasattr(sub_help, 'command_table'): @@ -40,9 +41,12 @@ def test_no_event_stream_unless_allowed(): continue supported_commands = '\n'.join(_ALLOWED_COMMANDS) errors.append( - 'The "%s" command uses event streams ' + f'The {full_command} command uses event streams ' 'which is only supported for these operations:\n' - '%s' % (full_command, supported_commands) + f'{supported_commands}\n\n' ) + targets.append(full_command) if errors: - raise AssertionError('\n' + '\n'.join(errors)) + raise AssertionError( + f"\n{'\n'.join(errors)}\nTarget=[{', '.join(targets)}]" + ) diff --git a/tests/functional/test_shadowing.py b/tests/functional/test_shadowing.py index 34060573226b..57d6a4426c69 100644 --- a/tests/functional/test_shadowing.py +++ b/tests/functional/test_shadowing.py @@ -56,6 +56,7 @@ def test_no_shadowed_builtins(command_name, command_table, builtins): """ errors = [] + targets=[] for sub_name, sub_command in command_table.items(): op_help = sub_command.create_help_command() arg_table = op_help.arg_table @@ -64,7 +65,10 @@ def test_no_shadowed_builtins(command_name, command_table, builtins): # Then we are shadowing or prefixing a top level argument errors.append( 'Shadowing/Prefixing a top level option: ' - '%s.%s.%s' % (command_name, sub_name, arg_name) + f'{command_name}.{sub_name}.{arg_name}' ) + targets.append(f'{command_name}.{sub_name}.{arg_name}') if errors: - raise AssertionError('\n' + '\n'.join(errors)) + raise AssertionError( + f'\n{'\n'.join(errors)}\nTarget=[{','.join(targets)}]' + ) From 2f28a184c74f546a4e23b39fb4213942315e6bb7 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 29 Oct 2025 12:57:10 -0400 Subject: [PATCH 02/13] Update tests to remove Code= --- tests/functional/botocore/test_paginator_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 252d6602a280..f2cba0f23514 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -167,7 +167,7 @@ def test_result_key_exists(operation_name, page_config, service_model): raise AssertionError( "Required key 'result_key' is missing " f"from pagination config: {page_config}\n" - f"Code=ResultKeyMissing\nTarget={operation_name}.result_key" + f"Target={operation_name}.result_key" ) From 44f1c09802177557ba71faaa2417ca1b4a583c27 Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 29 Oct 2025 12:58:25 -0400 Subject: [PATCH 03/13] Fix formatting to be consistent with other tests. --- tests/functional/botocore/test_supported_protocols.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functional/botocore/test_supported_protocols.py b/tests/functional/botocore/test_supported_protocols.py index 076fe1c0d46c..f34b0be058ef 100644 --- a/tests/functional/botocore/test_supported_protocols.py +++ b/tests/functional/botocore/test_supported_protocols.py @@ -54,8 +54,8 @@ def test_services_with_protocols_trait_have_supported_protocol( service_name, supported_protocols ): message = ( - f"No protocols supported for service {service_name}" - f"\nTarget={service_name}" + f"No protocols supported for service {service_name}\n" + f"Target={service_name}" ) assert any( protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS @@ -72,7 +72,7 @@ def test_services_without_protocols_trait_have_supported_protocol( service_name, supported_protocol ): message = ( - f"Service protocol not supported for {service_name}" - f"\nTarget={service_name}" + f"Service protocol not supported for {service_name}\n" + f"Target={service_name}" ) assert supported_protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS, message From c3d6462c205c183f5b92500966dc0aa27a38eecc Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 29 Oct 2025 15:27:55 -0400 Subject: [PATCH 04/13] Revert some unneeded changes. --- .../botocore/test_paginator_config.py | 50 +++++++------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index f2cba0f23514..0da5135d1a70 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -149,7 +149,17 @@ def _pagination_configs(): @pytest.mark.parametrize( "operation_name, page_config, service_model", _pagination_configs() ) -def test_known_pagination_keys(operation_name, page_config, service_model): +def test_lint_pagination_configs(operation_name, page_config, service_model): + _validate_known_pagination_keys(page_config) + _validate_result_key_exists(operation_name, page_config) + _validate_referenced_operation_exists(operation_name, service_model) + _validate_operation_has_output(operation_name, service_model) + _validate_input_keys_match(operation_name, page_config, service_model) + _validate_output_keys_match(operation_name, page_config, service_model) + _validate_new_numeric_keys(operation_name, page_config, service_model) + + +def _validate_known_pagination_keys(operation_name, page_config): for key in page_config: if key not in KNOWN_PAGE_KEYS: raise AssertionError( @@ -158,11 +168,7 @@ def test_known_pagination_keys(operation_name, page_config, service_model): ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_result_key_exists(operation_name, page_config, service_model): +def _validate_result_key_exists(operation_name, page_config): if 'result_key' not in page_config: raise AssertionError( "Required key 'result_key' is missing " @@ -171,11 +177,7 @@ def test_result_key_exists(operation_name, page_config, service_model): ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_referenced_operation_exists(operation_name, page_config, service_model): +def _validate_referenced_operation_exists(operation_name, service_model): if operation_name not in service_model.operation_names: raise AssertionError( "Pagination config refers to operation that " @@ -184,11 +186,7 @@ def test_referenced_operation_exists(operation_name, page_config, service_model) ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_operation_has_output(operation_name, page_config, service_model): +def _validate_operation_has_output(operation_name, service_model): op_model = service_model.operation_model(operation_name) output = op_model.output_shape if output is None or not output.members: @@ -199,11 +197,7 @@ def test_operation_has_output(operation_name, page_config, service_model): ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_input_keys_match(operation_name, page_config, service_model): +def _validate_input_keys_match(operation_name, page_config, service_model): input_tokens = page_config['input_token'] if not isinstance(input_tokens, list): input_tokens = [input_tokens] @@ -229,11 +223,7 @@ def test_input_keys_match(operation_name, page_config, service_model): ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_output_keys_match(operation_name, page_config, service_model): +def _validate_output_keys_match(operation_name, page_config, service_model): # NOTE: The original version of this function from translate.py had logic # to ensure that the entire set of output_members was accounted for in the # union of 'result_key', 'output_token', 'more_results', and @@ -274,11 +264,7 @@ def test_output_keys_match(operation_name, page_config, service_model): ) -@pytest.mark.validates_models -@pytest.mark.parametrize( - "operation_name, page_config, service_model", _pagination_configs() -) -def test_new_numeric_keys(operation_name, page_config, service_model): +def _validate_new_numeric_keys(operation_name, page_config, service_model): output_shape = service_model.operation_model(operation_name).output_shape for key in _get_list_value(page_config, 'result_key'): current_shape = output_shape @@ -299,7 +285,7 @@ def test_new_numeric_keys(operation_name, page_config, service_model): 'integer outputs across pages. Verify that this behavior is ' 'correct before allow-listing, since whether or not it is ' 'appropriate to sum depends on the subject matter.\n' - f'Target={service_model.service_name}.{operation_name}' + f'Target={operation_name}' ) From 681043807fc7b5edf7e0b016d1a5954d3634bcbe Mon Sep 17 00:00:00 2001 From: aemous Date: Wed, 29 Oct 2025 15:32:45 -0400 Subject: [PATCH 05/13] Add missing arg. --- tests/functional/botocore/test_paginator_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 0da5135d1a70..b3b9ebb57734 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -150,7 +150,7 @@ def _pagination_configs(): "operation_name, page_config, service_model", _pagination_configs() ) def test_lint_pagination_configs(operation_name, page_config, service_model): - _validate_known_pagination_keys(page_config) + _validate_known_pagination_keys(operation_name, page_config) _validate_result_key_exists(operation_name, page_config) _validate_referenced_operation_exists(operation_name, service_model) _validate_operation_has_output(operation_name, service_model) From 0e8c07559271b02fafc6bc05e267e5f87dbd614a Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 30 Oct 2025 09:40:10 -0400 Subject: [PATCH 06/13] Fix usage of quotes in f-strings. --- tests/functional/test_shadowing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_shadowing.py b/tests/functional/test_shadowing.py index 57d6a4426c69..01e3f85271af 100644 --- a/tests/functional/test_shadowing.py +++ b/tests/functional/test_shadowing.py @@ -70,5 +70,5 @@ def test_no_shadowed_builtins(command_name, command_table, builtins): targets.append(f'{command_name}.{sub_name}.{arg_name}') if errors: raise AssertionError( - f'\n{'\n'.join(errors)}\nTarget=[{','.join(targets)}]' + '\n' + '\n'.join(errors) + '\nTarget=[' + ','.join(targets) + ']' ) From 687838adaaf0a5968ca2dc44738a957fcbfdc1e0 Mon Sep 17 00:00:00 2001 From: aemous Date: Thu, 30 Oct 2025 11:02:24 -0400 Subject: [PATCH 07/13] Fix quote in f-string. --- tests/functional/test_no_event_streams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_no_event_streams.py b/tests/functional/test_no_event_streams.py index 68330809aa28..7edf188b6ce8 100644 --- a/tests/functional/test_no_event_streams.py +++ b/tests/functional/test_no_event_streams.py @@ -48,5 +48,5 @@ def test_no_event_stream_unless_allowed(): targets.append(full_command) if errors: raise AssertionError( - f"\n{'\n'.join(errors)}\nTarget=[{', '.join(targets)}]" + '\n' + '\n'.join(errors) + '\nTarget=[' + ', '.join(targets) + ']' ) From 2f9d53ad48b01271aca90ad439bccc779b8b2eab Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 11:22:29 -0500 Subject: [PATCH 08/13] Switch to using PyTest custom properties. --- tests/functional/botocore/test_h2_required.py | 19 +++---- .../botocore/test_paginator_config.py | 56 +++++++++---------- .../botocore/test_supported_protocols.py | 14 +++-- tests/functional/test_no_event_streams.py | 20 ++++--- tests/functional/test_shadowing.py | 26 +++++++-- 5 files changed, 76 insertions(+), 59 deletions(-) diff --git a/tests/functional/botocore/test_h2_required.py b/tests/functional/botocore/test_h2_required.py index 72f446c2e774..8487a6b0dec8 100644 --- a/tests/functional/botocore/test_h2_required.py +++ b/tests/functional/botocore/test_h2_required.py @@ -53,23 +53,22 @@ def _all_test_cases(): @pytest.mark.validates_models @pytest.mark.parametrize("h2_service", H2_SERVICES) -def test_all_uses_of_h2_are_known(h2_service): +def test_all_uses_of_h2_are_known(h2_service, record_property): # Validates that a service that requires HTTP 2 for all operations is known - message = ( - f'Found unknown HTTP 2 service: {h2_service}\n' - f'Target={h2_service}' - ) + message = f'Found unknown HTTP 2 service: {h2_service}' + # Store the service name in a PyTest custom property + record_property('aws_service', h2_service) assert _KNOWN_SERVICES.get(h2_service) is _H2_REQUIRED, message @pytest.mark.validates_models @pytest.mark.parametrize("h2_service, operation", H2_OPERATIONS) -def test_all_h2_operations_are_known(h2_service, operation): +def test_all_h2_operations_are_known(h2_service, operation, record_property): # Validates that an operation that requires HTTP 2 is known known_operations = _KNOWN_SERVICES.get(h2_service, []) - message = ( - f'Found unknown HTTP 2 operation: {h2_service}.{operation}\n' - f"Target={h2_service}.{operation}" - ) + message = f'Found unknown HTTP 2 operation: {h2_service}.{operation}' + # Store the service name and operation in PyTest custom properties + record_property('aws_service', h2_service) + record_property('aws_operation', operation) assert operation in known_operations, message diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index b3b9ebb57734..39b5ff1717ad 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -149,31 +149,32 @@ def _pagination_configs(): @pytest.mark.parametrize( "operation_name, page_config, service_model", _pagination_configs() ) -def test_lint_pagination_configs(operation_name, page_config, service_model): - _validate_known_pagination_keys(operation_name, page_config) - _validate_result_key_exists(operation_name, page_config) +def test_lint_pagination_configs(operation_name, page_config, service_model, record_property): + # Store common details of the operation + record_property('aws_service', service_model.service_id) + record_property('aws_operation', operation_name) + _validate_known_pagination_keys(page_config) + _validate_result_key_exists(page_config) _validate_referenced_operation_exists(operation_name, service_model) - _validate_operation_has_output(operation_name, service_model) + _validate_operation_has_output(operation_name, service_model, record_property) _validate_input_keys_match(operation_name, page_config, service_model) - _validate_output_keys_match(operation_name, page_config, service_model) - _validate_new_numeric_keys(operation_name, page_config, service_model) + _validate_output_keys_match(operation_name, page_config, service_model, record_property) + _validate_new_numeric_keys(operation_name, page_config, service_model, record_property) -def _validate_known_pagination_keys(operation_name, page_config): +def _validate_known_pagination_keys(page_config): for key in page_config: if key not in KNOWN_PAGE_KEYS: raise AssertionError( - f"Unknown key '{key}' in pagination config: {page_config}\n" - f"Target={operation_name}.{key}" + f"Unknown key '{key}' in pagination config: {page_config}" ) -def _validate_result_key_exists(operation_name, page_config): +def _validate_result_key_exists(page_config): if 'result_key' not in page_config: raise AssertionError( "Required key 'result_key' is missing " - f"from pagination config: {page_config}\n" - f"Target={operation_name}.result_key" + f"from pagination config: {page_config}" ) @@ -181,19 +182,18 @@ def _validate_referenced_operation_exists(operation_name, service_model): if operation_name not in service_model.operation_names: raise AssertionError( "Pagination config refers to operation that " - f"does not exist: {operation_name}\n" - f"Target={operation_name}" + f"does not exist: {operation_name}" ) -def _validate_operation_has_output(operation_name, service_model): +def _validate_operation_has_output(operation_name, service_model, record_property): op_model = service_model.operation_model(operation_name) output = op_model.output_shape + record_property('shape', output.type_name) if output is None or not output.members: raise AssertionError( "Pagination config refers to operation " - f"that does not have any output: {operation_name}\n" - f"Target={operation_name}" + f"that does not have any output: {operation_name}" ) @@ -208,8 +208,7 @@ def _validate_input_keys_match(operation_name, page_config, service_model): if token not in valid_input_names: raise AssertionError( f"input_token refers to a non existent " - f"input member for operation.\n" - f"Target={operation_name}.{token}" + f"input member for operation." ) if 'limit_key' in page_config: limit_key = page_config['limit_key'] @@ -218,12 +217,11 @@ def _validate_input_keys_match(operation_name, page_config, service_model): raise AssertionError( f"limit_key '{limit_key}' refers to a non existent " f"input member for operation: {operation_name}, valid keys: " - f"{valid_keys}.\n" - f"Target={operation_name}.{limit_key}" + f"{valid_keys}." ) -def _validate_output_keys_match(operation_name, page_config, service_model): +def _validate_output_keys_match(operation_name, page_config, service_model, record_property): # NOTE: The original version of this function from translate.py had logic # to ensure that the entire set of output_members was accounted for in the # union of 'result_key', 'output_token', 'more_results', and @@ -239,9 +237,9 @@ def _validate_output_keys_match(operation_name, page_config, service_model): else: if output_key not in output_members: raise AssertionError( - f"Pagination key '{key_name}' refers to an output " - f"member that does not exist: {output_key}\n" - f"Target={operation_name}.{output_key}" + f"Pagination key '{key_name}' for operation " + f"{operation_name} refers to an output " + f"member that does not exist: {output_key}" ) output_members.remove(output_key) @@ -259,12 +257,11 @@ def _validate_output_keys_match(operation_name, page_config, service_model): "There are member names in the output shape of " f"{operation_name} that are not accounted for in the pagination " f"config for service {service_model.service_name}: " - f"[{', '.join(output_members)}]\n" - f"Target={operation_name}[{', '.join(output_members)}]" + f"[{', '.join(output_members)}]" ) -def _validate_new_numeric_keys(operation_name, page_config, service_model): +def _validate_new_numeric_keys(operation_name, page_config, service_model, record_property): output_shape = service_model.operation_model(operation_name).output_shape for key in _get_list_value(page_config, 'result_key'): current_shape = output_shape @@ -284,8 +281,7 @@ def _validate_new_numeric_keys(operation_name, page_config, service_model): f'{service_model.service_name} that is configured to sum ' 'integer outputs across pages. Verify that this behavior is ' 'correct before allow-listing, since whether or not it is ' - 'appropriate to sum depends on the subject matter.\n' - f'Target={operation_name}' + 'appropriate to sum depends on the subject matter.' ) diff --git a/tests/functional/botocore/test_supported_protocols.py b/tests/functional/botocore/test_supported_protocols.py index f34b0be058ef..bc5cdf5e7dce 100644 --- a/tests/functional/botocore/test_supported_protocols.py +++ b/tests/functional/botocore/test_supported_protocols.py @@ -51,12 +51,13 @@ def _single_protocol_test_cases(): _multi_protocol_test_cases(), ) def test_services_with_protocols_trait_have_supported_protocol( - service_name, supported_protocols + service_name, supported_protocols, record_property ): message = ( - f"No protocols supported for service {service_name}\n" - f"Target={service_name}" + f"No protocols supported for service {service_name}" ) + # Store the service name in PyTest custom properties + record_property('aws_service', service_name) assert any( protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS for protocol in supported_protocols @@ -69,10 +70,11 @@ def test_services_with_protocols_trait_have_supported_protocol( _single_protocol_test_cases(), ) def test_services_without_protocols_trait_have_supported_protocol( - service_name, supported_protocol + service_name, supported_protocol, record_property ): message = ( - f"Service protocol not supported for {service_name}\n" - f"Target={service_name}" + f"Service protocol not supported for {service_name}" ) + # Store the service name in PyTest custom properties + record_property('aws_service', service_name) assert supported_protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS, message diff --git a/tests/functional/test_no_event_streams.py b/tests/functional/test_no_event_streams.py index 7edf188b6ce8..6e4b81f6aaea 100644 --- a/tests/functional/test_no_event_streams.py +++ b/tests/functional/test_no_event_streams.py @@ -20,11 +20,10 @@ @pytest.mark.validates_models -def test_no_event_stream_unless_allowed(): +def test_no_event_stream_unless_allowed(record_property): driver = create_clidriver() help_command = driver.create_help_command() errors = [] - targets = [] for command_name, command_obj in help_command.command_table.items(): sub_help = command_obj.create_help_command() if hasattr(sub_help, 'command_table'): @@ -39,14 +38,21 @@ def test_no_event_stream_unless_allowed(): ): if full_command in _ALLOWED_COMMANDS: continue + # Store the service and operation in + # PyTest custom properties + record_property( + 'aws_service', + model.service_model.service_id + ) + record_property( + 'aws_operation', + model.name + ) supported_commands = '\n'.join(_ALLOWED_COMMANDS) errors.append( f'The {full_command} command uses event streams ' 'which is only supported for these operations:\n' - f'{supported_commands}\n\n' + f'{supported_commands}' ) - targets.append(full_command) if errors: - raise AssertionError( - '\n' + '\n'.join(errors) + '\nTarget=[' + ', '.join(targets) + ']' - ) + raise AssertionError('\n' + '\n'.join(errors)) diff --git a/tests/functional/test_shadowing.py b/tests/functional/test_shadowing.py index 01e3f85271af..01b346cb9904 100644 --- a/tests/functional/test_shadowing.py +++ b/tests/functional/test_shadowing.py @@ -11,6 +11,7 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import pytest +from botocore.model import OperationModel from awscli.clidriver import create_clidriver @@ -29,7 +30,12 @@ def _generate_command_tests(): @pytest.mark.parametrize( "command_name, command_table, builtins", _generate_command_tests() ) -def test_no_shadowed_builtins(command_name, command_table, builtins): +def test_no_shadowed_builtins( + command_name, + command_table, + builtins, + record_property +): """Verify no command params are shadowed or prefixed by the built in param. The CLI parses all command line options into a single namespace. @@ -56,19 +62,27 @@ def test_no_shadowed_builtins(command_name, command_table, builtins): """ errors = [] - targets=[] for sub_name, sub_command in command_table.items(): op_help = sub_command.create_help_command() + model = op_help.obj arg_table = op_help.arg_table for arg_name in arg_table: if any(p.startswith(arg_name) for p in builtins): + if isinstance(model, OperationModel): + # Store the service and operation in + # PyTest custom properties + record_property( + 'aws_service', + model.service_model.service_id + ) + record_property( + 'aws_operation', + model.name + ) # Then we are shadowing or prefixing a top level argument errors.append( 'Shadowing/Prefixing a top level option: ' f'{command_name}.{sub_name}.{arg_name}' ) - targets.append(f'{command_name}.{sub_name}.{arg_name}') if errors: - raise AssertionError( - '\n' + '\n'.join(errors) + '\nTarget=[' + ','.join(targets) + ']' - ) + raise AssertionError('\n' + '\n'.join(errors)) From 47cb7a95306d0a46404a46b809d8daddd4f5d535 Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 11:31:08 -0500 Subject: [PATCH 09/13] Minor fixes. --- tests/functional/botocore/test_h2_required.py | 1 - tests/functional/botocore/test_paginator_config.py | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/functional/botocore/test_h2_required.py b/tests/functional/botocore/test_h2_required.py index 8487a6b0dec8..c22b39db7ddf 100644 --- a/tests/functional/botocore/test_h2_required.py +++ b/tests/functional/botocore/test_h2_required.py @@ -70,5 +70,4 @@ def test_all_h2_operations_are_known(h2_service, operation, record_property): # Store the service name and operation in PyTest custom properties record_property('aws_service', h2_service) record_property('aws_operation', operation) - assert operation in known_operations, message diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 39b5ff1717ad..2d61dbee34dd 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -158,7 +158,7 @@ def test_lint_pagination_configs(operation_name, page_config, service_model, rec _validate_referenced_operation_exists(operation_name, service_model) _validate_operation_has_output(operation_name, service_model, record_property) _validate_input_keys_match(operation_name, page_config, service_model) - _validate_output_keys_match(operation_name, page_config, service_model, record_property) + _validate_output_keys_match(operation_name, page_config, service_model) _validate_new_numeric_keys(operation_name, page_config, service_model, record_property) @@ -207,21 +207,20 @@ def _validate_input_keys_match(operation_name, page_config, service_model): for token in input_tokens: if token not in valid_input_names: raise AssertionError( - f"input_token refers to a non existent " - f"input member for operation." + f"input_token '{token}' refers to a non existent " + f"input member for operation: {operation_name}" ) if 'limit_key' in page_config: limit_key = page_config['limit_key'] if limit_key not in valid_input_names: - valid_keys = ', '.join(list(valid_input_names)) raise AssertionError( f"limit_key '{limit_key}' refers to a non existent " f"input member for operation: {operation_name}, valid keys: " - f"{valid_keys}." + f"{', '.join(list(valid_input_names))}." ) -def _validate_output_keys_match(operation_name, page_config, service_model, record_property): +def _validate_output_keys_match(operation_name, page_config, service_model): # NOTE: The original version of this function from translate.py had logic # to ensure that the entire set of output_members was accounted for in the # union of 'result_key', 'output_token', 'more_results', and @@ -257,7 +256,7 @@ def _validate_output_keys_match(operation_name, page_config, service_model, reco "There are member names in the output shape of " f"{operation_name} that are not accounted for in the pagination " f"config for service {service_model.service_name}: " - f"[{', '.join(output_members)}]" + f"{', '.join(output_members)}" ) From 0dbae3e5015391dc8843e18171ca9ae018b5b768 Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 11:39:15 -0500 Subject: [PATCH 10/13] Minor updates. --- tests/functional/botocore/test_paginator_config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 2d61dbee34dd..3d6606205ef8 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -189,8 +189,9 @@ def _validate_referenced_operation_exists(operation_name, service_model): def _validate_operation_has_output(operation_name, service_model, record_property): op_model = service_model.operation_model(operation_name) output = op_model.output_shape - record_property('shape', output.type_name) if output is None or not output.members: + if output: + record_property('shape', output.type_name) raise AssertionError( "Pagination config refers to operation " f"that does not have any output: {operation_name}" @@ -275,6 +276,7 @@ def _validate_new_numeric_keys(operation_name, page_config, service_model, recor and (service_model.service_name, operation_name) not in KNOWN_PAGINATORS_WITH_INTEGER_OUTPUTS ): + record_property('shape', current_shape.name) raise AssertionError( f'There is a new operation {operation_name} for service ' f'{service_model.service_name} that is configured to sum ' From 5b31589bb33b100e00f57f6d9cb40e3ac2164ead Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 11:47:21 -0500 Subject: [PATCH 11/13] Formatting. --- tests/functional/botocore/test_supported_protocols.py | 8 ++------ tests/functional/test_no_event_streams.py | 2 +- tests/functional/test_shadowing.py | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/functional/botocore/test_supported_protocols.py b/tests/functional/botocore/test_supported_protocols.py index bc5cdf5e7dce..901202e47a88 100644 --- a/tests/functional/botocore/test_supported_protocols.py +++ b/tests/functional/botocore/test_supported_protocols.py @@ -53,9 +53,7 @@ def _single_protocol_test_cases(): def test_services_with_protocols_trait_have_supported_protocol( service_name, supported_protocols, record_property ): - message = ( - f"No protocols supported for service {service_name}" - ) + message = f"No protocols supported for service {service_name}" # Store the service name in PyTest custom properties record_property('aws_service', service_name) assert any( @@ -72,9 +70,7 @@ def test_services_with_protocols_trait_have_supported_protocol( def test_services_without_protocols_trait_have_supported_protocol( service_name, supported_protocol, record_property ): - message = ( - f"Service protocol not supported for {service_name}" - ) + message = f"Service protocol not supported for {service_name}" # Store the service name in PyTest custom properties record_property('aws_service', service_name) assert supported_protocol in PRIORITY_ORDERED_SUPPORTED_PROTOCOLS, message diff --git a/tests/functional/test_no_event_streams.py b/tests/functional/test_no_event_streams.py index 6e4b81f6aaea..d41a9952e711 100644 --- a/tests/functional/test_no_event_streams.py +++ b/tests/functional/test_no_event_streams.py @@ -31,7 +31,7 @@ def test_no_event_stream_unless_allowed(record_property): op_help = sub_command.create_help_command() model = op_help.obj if isinstance(model, OperationModel): - full_command = '%s %s' % (command_name, sub_name) + full_command = f'{command_name} {sub_name}' if ( model.has_event_stream_input or model.has_event_stream_output diff --git a/tests/functional/test_shadowing.py b/tests/functional/test_shadowing.py index 01b346cb9904..af43508ed973 100644 --- a/tests/functional/test_shadowing.py +++ b/tests/functional/test_shadowing.py @@ -11,8 +11,8 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import pytest -from botocore.model import OperationModel +from botocore.model import OperationModel from awscli.clidriver import create_clidriver From 9e087f4dedf3ef6852fe893219a1ab1787ac6378 Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 11:48:21 -0500 Subject: [PATCH 12/13] Run formatters. --- tests/functional/botocore/test_h2_required.py | 4 ++-- .../botocore/test_paginator_config.py | 20 ++++++++++++++----- tests/functional/test_no_event_streams.py | 8 ++------ tests/functional/test_shadowing.py | 15 ++++---------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/tests/functional/botocore/test_h2_required.py b/tests/functional/botocore/test_h2_required.py index c22b39db7ddf..c0673d8116a7 100644 --- a/tests/functional/botocore/test_h2_required.py +++ b/tests/functional/botocore/test_h2_required.py @@ -19,8 +19,8 @@ 'qbusiness': ['Chat'], 'kinesis': ['SubscribeToShard'], 'lexv2-runtime': ['StartConversation'], - # Added only to keep a record of this feature being incompatible - 'bedrock-runtime': ['InvokeModelWithBidirectionalStream'], + # Added only to keep a record of this feature being incompatible + 'bedrock-runtime': ['InvokeModelWithBidirectionalStream'], } diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index 3d6606205ef8..c2c428d1f6aa 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -149,17 +149,23 @@ def _pagination_configs(): @pytest.mark.parametrize( "operation_name, page_config, service_model", _pagination_configs() ) -def test_lint_pagination_configs(operation_name, page_config, service_model, record_property): +def test_lint_pagination_configs( + operation_name, page_config, service_model, record_property +): # Store common details of the operation record_property('aws_service', service_model.service_id) record_property('aws_operation', operation_name) _validate_known_pagination_keys(page_config) _validate_result_key_exists(page_config) _validate_referenced_operation_exists(operation_name, service_model) - _validate_operation_has_output(operation_name, service_model, record_property) + _validate_operation_has_output( + operation_name, service_model, record_property + ) _validate_input_keys_match(operation_name, page_config, service_model) _validate_output_keys_match(operation_name, page_config, service_model) - _validate_new_numeric_keys(operation_name, page_config, service_model, record_property) + _validate_new_numeric_keys( + operation_name, page_config, service_model, record_property + ) def _validate_known_pagination_keys(page_config): @@ -186,7 +192,9 @@ def _validate_referenced_operation_exists(operation_name, service_model): ) -def _validate_operation_has_output(operation_name, service_model, record_property): +def _validate_operation_has_output( + operation_name, service_model, record_property +): op_model = service_model.operation_model(operation_name) output = op_model.output_shape if output is None or not output.members: @@ -261,7 +269,9 @@ def _validate_output_keys_match(operation_name, page_config, service_model): ) -def _validate_new_numeric_keys(operation_name, page_config, service_model, record_property): +def _validate_new_numeric_keys( + operation_name, page_config, service_model, record_property +): output_shape = service_model.operation_model(operation_name).output_shape for key in _get_list_value(page_config, 'result_key'): current_shape = output_shape diff --git a/tests/functional/test_no_event_streams.py b/tests/functional/test_no_event_streams.py index d41a9952e711..72ae95ff8976 100644 --- a/tests/functional/test_no_event_streams.py +++ b/tests/functional/test_no_event_streams.py @@ -41,13 +41,9 @@ def test_no_event_stream_unless_allowed(record_property): # Store the service and operation in # PyTest custom properties record_property( - 'aws_service', - model.service_model.service_id - ) - record_property( - 'aws_operation', - model.name + 'aws_service', model.service_model.service_id ) + record_property('aws_operation', model.name) supported_commands = '\n'.join(_ALLOWED_COMMANDS) errors.append( f'The {full_command} command uses event streams ' diff --git a/tests/functional/test_shadowing.py b/tests/functional/test_shadowing.py index af43508ed973..4ae8c89ba069 100644 --- a/tests/functional/test_shadowing.py +++ b/tests/functional/test_shadowing.py @@ -11,8 +11,8 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import pytest - from botocore.model import OperationModel + from awscli.clidriver import create_clidriver @@ -31,10 +31,7 @@ def _generate_command_tests(): "command_name, command_table, builtins", _generate_command_tests() ) def test_no_shadowed_builtins( - command_name, - command_table, - builtins, - record_property + command_name, command_table, builtins, record_property ): """Verify no command params are shadowed or prefixed by the built in param. @@ -72,13 +69,9 @@ def test_no_shadowed_builtins( # Store the service and operation in # PyTest custom properties record_property( - 'aws_service', - model.service_model.service_id - ) - record_property( - 'aws_operation', - model.name + 'aws_service', model.service_model.service_id ) + record_property('aws_operation', model.name) # Then we are shadowing or prefixing a top level argument errors.append( 'Shadowing/Prefixing a top level option: ' From 7441ca1d00dadab31639772edcbcd0eb23479676 Mon Sep 17 00:00:00 2001 From: aemous Date: Mon, 3 Nov 2025 13:51:34 -0500 Subject: [PATCH 13/13] Replace service_id with service_name for serialization capabilities. --- tests/functional/botocore/test_paginator_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/botocore/test_paginator_config.py b/tests/functional/botocore/test_paginator_config.py index c2c428d1f6aa..e77371ae31e3 100644 --- a/tests/functional/botocore/test_paginator_config.py +++ b/tests/functional/botocore/test_paginator_config.py @@ -153,7 +153,7 @@ def test_lint_pagination_configs( operation_name, page_config, service_model, record_property ): # Store common details of the operation - record_property('aws_service', service_model.service_id) + record_property('aws_service', service_model.service_name) record_property('aws_operation', operation_name) _validate_known_pagination_keys(page_config) _validate_result_key_exists(page_config)