From be26c8f1a0d2aad1b17fdbf6f4aa7f092da697b5 Mon Sep 17 00:00:00 2001 From: Steve Yoo Date: Wed, 23 Apr 2025 14:57:38 -0400 Subject: [PATCH] Reduce invalid choice output --- awscli/argparser.py | 17 ++++------------- tests/functional/docs/test_help_output.py | 2 +- tests/functional/ec2instanceconnect/test_ssh.py | 2 +- .../kinesis/test_remove_operations.py | 2 +- tests/functional/lex/test_remove_operations.py | 2 +- .../test_generate_createproduct.py | 4 ++-- .../test_generate_createprovisioningartifact.py | 2 +- tests/unit/test_clidriver.py | 4 ++-- 8 files changed, 13 insertions(+), 22 deletions(-) diff --git a/awscli/argparser.py b/awscli/argparser.py index 2a7f57e35fd8..6e1bd0478470 100644 --- a/awscli/argparser.py +++ b/awscli/argparser.py @@ -65,10 +65,6 @@ def choices(self, val): class CLIArgParser(argparse.ArgumentParser): Formatter = argparse.RawTextHelpFormatter - # When displaying invalid choice error messages, - # this controls how many options to show per line. - ChoicesPerLine = 2 - def _check_value(self, action, value): """ It's probably not a great idea to override a "hidden" method @@ -77,15 +73,10 @@ def _check_value(self, action, value): """ # converted value must be one of the choices (if specified) if action.choices is not None and value not in action.choices: - msg = ['Invalid choice, valid choices are:\n'] - for i in range(len(action.choices))[:: self.ChoicesPerLine]: - current = [] - for choice in action.choices[i : i + self.ChoicesPerLine]: - current.append('%-40s' % choice) - msg.append(' | '.join(current)) + msg = [f"Found invalid choice '{value}'\n"] possible = get_close_matches(value, action.choices, cutoff=0.8) if possible: - extra = ['\n\nInvalid choice: %r, maybe you meant:\n' % value] + extra = ['Maybe you meant:\n'] for word in possible: extra.append(' * %s' % word) msg.extend(extra) @@ -126,8 +117,8 @@ def error(self, message): should raise an exception. """ usage_message = self.format_usage() - error_message = f'{self.prog}: error: {message}\n' - raise ArgParseException(f'{usage_message}\n{error_message}') + error_message = f'{self.prog}: error: {message}' + raise ArgParseException(f'{error_message}\n\n{usage_message}') class MainArgParser(CLIArgParser): diff --git a/tests/functional/docs/test_help_output.py b/tests/functional/docs/test_help_output.py index fff29801d0d8..902581b06397 100644 --- a/tests/functional/docs/test_help_output.py +++ b/tests/functional/docs/test_help_output.py @@ -217,7 +217,7 @@ def assert_command_does_not_exist(self, service, command): self.assertEqual(cr, 252) # We should see an error message complaining about # an invalid choice because the operation has been removed. - self.assertIn('argument operation: Invalid choice', stderr.getvalue()) + self.assertIn('argument operation: Found invalid choice', stderr.getvalue()) def test_ses_deprecated_commands(self): self.driver.main(['ses', 'help']) diff --git a/tests/functional/ec2instanceconnect/test_ssh.py b/tests/functional/ec2instanceconnect/test_ssh.py index 15b64261bd31..66ffcbd64af7 100644 --- a/tests/functional/ec2instanceconnect/test_ssh.py +++ b/tests/functional/ec2instanceconnect/test_ssh.py @@ -886,7 +886,7 @@ def test_command_fails_when_eice_connection_type_and_no_private_ip( "test", ], 252, - "argument --connection-type: Invalid choice, valid choices are:", + "argument --connection-type: Found invalid choice 'test'", id='Failure: Customer must provide connection-type when IP defined', ), pytest.param( diff --git a/tests/functional/kinesis/test_remove_operations.py b/tests/functional/kinesis/test_remove_operations.py index c7221f8565da..1d072dd4b091 100644 --- a/tests/functional/kinesis/test_remove_operations.py +++ b/tests/functional/kinesis/test_remove_operations.py @@ -15,5 +15,5 @@ def test_subscribe_to_shard_removed(): result = CLIRunner().run(['kinesis', 'subscribe-to-shard', 'help']) - expected_error = 'argument operation: Invalid choice, valid choices are:' + expected_error = "argument operation: Found invalid choice 'subscribe-to-shard'" assert expected_error in result.stderr diff --git a/tests/functional/lex/test_remove_operations.py b/tests/functional/lex/test_remove_operations.py index 74bd87cd27bd..9e0a0c82bf72 100644 --- a/tests/functional/lex/test_remove_operations.py +++ b/tests/functional/lex/test_remove_operations.py @@ -15,5 +15,5 @@ def test_start_conversation_removed(): result = CLIRunner().run(['lexv2-runtime', 'start-conversation', 'help']) - expected_error = 'argument operation: Invalid choice, valid choices are:' + expected_error = "argument operation: Found invalid choice 'start-conversation'" assert expected_error in result.stderr diff --git a/tests/functional/servicecatalog/test_generate_createproduct.py b/tests/functional/servicecatalog/test_generate_createproduct.py index eee7f5630af6..f3a5d8b6e3a5 100644 --- a/tests/functional/servicecatalog/test_generate_createproduct.py +++ b/tests/functional/servicecatalog/test_generate_createproduct.py @@ -198,7 +198,7 @@ def test_invalid_product_type(self): self.assert_params_for_cmd( self.cmd_line, expected_rc=252, - stderr_contains='--product-type: Invalid choice', + stderr_contains='--product-type: Found invalid choice', ) def test_generate_product_invalid_provisioning_artifact_type(self): @@ -208,5 +208,5 @@ def test_generate_product_invalid_provisioning_artifact_type(self): self.assert_params_for_cmd( self.cmd_line, expected_rc=252, - stderr_contains='--provisioning-artifact-type: Invalid choice', + stderr_contains='--provisioning-artifact-type: Found invalid choice', ) diff --git a/tests/functional/servicecatalog/test_generate_createprovisioningartifact.py b/tests/functional/servicecatalog/test_generate_createprovisioningartifact.py index 3daa9b355e06..305bd5d63609 100644 --- a/tests/functional/servicecatalog/test_generate_createprovisioningartifact.py +++ b/tests/functional/servicecatalog/test_generate_createprovisioningartifact.py @@ -105,7 +105,7 @@ def test_generate_provisioning_artifact_invalid_pa_type(self): self.assert_params_for_cmd( self.cmd_line, expected_rc=252, - stderr_contains='--provisioning-artifact-type: Invalid choice', + stderr_contains='--provisioning-artifact-type: Found invalid choice', ) def test_generate_provisioning_artifact_missing_file_path(self): diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 67838fa7165a..a3f07e278dc2 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -493,10 +493,10 @@ def test_unknown_command_suggests_help(self): ) self.assertEqual(rc, 252) # Tell the user what went wrong. - self.assertIn("Invalid choice: 'list-objecst'", self.stderr.getvalue()) + self.assertIn("Found invalid choice 'list-objecst'", self.stderr.getvalue()) # Offer the user a suggestion. self.assertIn( - "maybe you meant:\n\n * list-objects", self.stderr.getvalue() + "Maybe you meant:\n\n * list-objects", self.stderr.getvalue() )