From b65f1b866826b366cb431cd7febe3a24f12157e0 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Thu, 26 Jun 2014 15:56:49 -0700 Subject: [PATCH] Don't validate --endpoint-url for help subcommand "aws cloudsearchdomain" has --endpoint-url validation because it's a required option for that service. However, we should *not* be validating this when a user is just trying to get help. The fix is to move the validation to the operation-args-parsed.* event, which is after all the help processing code. I've also added integration tests that actually run the help command for cloudsearchdomain to verify this behavior works. --- awscli/customizations/cloudsearchdomain.py | 8 ++++---- awscli/help.py | 2 +- tests/integration/test_cli.py | 17 +++++++++++++++++ .../customizations/test_cloudsearchdomain.py | 19 +++++++++++++++---- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/awscli/customizations/cloudsearchdomain.py b/awscli/customizations/cloudsearchdomain.py index 533c6fe8a512..c21ce9d151c0 100644 --- a/awscli/customizations/cloudsearchdomain.py +++ b/awscli/customizations/cloudsearchdomain.py @@ -19,11 +19,11 @@ """ def register_cloudsearchdomain(cli): - cli.register('top-level-args-parsed', validate_endpoint_url) + cli.register('operation-args-parsed.cloudsearchdomain', + validate_endpoint_url) -def validate_endpoint_url(parsed_args, **kwargs): - if parsed_args.command == 'cloudsearchdomain' and \ - parsed_args.endpoint_url is None: +def validate_endpoint_url(parsed_globals, **kwargs): + if parsed_globals.endpoint_url is None: raise ValueError( "--endpoint-url is required for cloudsearchdomain commands") diff --git a/awscli/help.py b/awscli/help.py index 25218a427b3f..5a695ed0b74c 100644 --- a/awscli/help.py +++ b/awscli/help.py @@ -114,7 +114,7 @@ def _exists_on_path(self, name): # Since we're only dealing with POSIX systems, we can # ignore things like PATHEXT. return any([os.path.exists(os.path.join(p, name)) - for p in os.environ.get('PATH', []).split(os.pathsep)]) + for p in os.environ.get('PATH', '').split(os.pathsep)]) def _popen(self, *args, **kwargs): return Popen(*args, **kwargs) diff --git a/tests/integration/test_cli.py b/tests/integration/test_cli.py index d96a524386f3..edbac3fa9fc2 100644 --- a/tests/integration/test_cli.py +++ b/tests/integration/test_cli.py @@ -76,6 +76,23 @@ def test_operation_help_with_required_arg(self): self.assertEqual(p.rc, 1, p.stderr) self.assertIn('get-object', p.stdout) + def test_service_help_with_required_option(self): + # In cloudsearchdomain, the --endpoint-url is required. + # We want to make sure if you're just getting help tex + # that we don't trigger that validation. + p = aws('cloudsearchdomain help') + self.assertEqual(p.rc, 1, p.stderr) + self.assertIn('cloudsearchdomain', p.stdout) + # And nothing on stderr about missing options. + self.assertEqual(p.stderr, '') + + def test_operation_help_with_required_option(self): + p = aws('cloudsearchdomain search help') + self.assertEqual(p.rc, 1, p.stderr) + self.assertIn('search', p.stdout) + # And nothing on stderr about missing options. + self.assertEqual(p.stderr, '') + def test_help_with_warning_blocks(self): p = aws('elastictranscoder create-pipeline help') self.assertEqual(p.rc, 1, p.stderr) diff --git a/tests/unit/customizations/test_cloudsearchdomain.py b/tests/unit/customizations/test_cloudsearchdomain.py index 8ba25137c980..211ccd404e2a 100644 --- a/tests/unit/customizations/test_cloudsearchdomain.py +++ b/tests/unit/customizations/test_cloudsearchdomain.py @@ -12,6 +12,7 @@ # language governing permissions and limitations under the License. from awscli.testutils import unittest from awscli.testutils import BaseAWSCommandParamsTest +from awscli.help import HelpRenderer from awscli.customizations.cloudsearchdomain import validate_endpoint_url import mock @@ -48,14 +49,24 @@ def test_endpoint_is_required(self): stderr = self.run_cmd(cmd, expected_rc=255)[1] self.assertIn('--endpoint-url is required', stderr) + def test_endpoint_not_required_for_help(self): + cmd = self.prefix + 'help' + with mock.patch('awscli.help.get_renderer') as get_renderer: + mock_render = mock.Mock(spec=HelpRenderer) + get_renderer.return_value = mock_render + stdout, stderr, rc = self.run_cmd(cmd, expected_rc=None) + # If we get this far we've succeeded, but we can do + # a quick sanity check and make sure the service name is + # in the stdout help text. + self.assertIn(stdout, 'cloudsearchdomain') + class TestCloudsearchDomainHandler(unittest.TestCase): def test_validate_endpoint_url_is_none(self): - parsed_args = mock.Mock() - parsed_args.endpoint_url = None - parsed_args.command = 'cloudsearchdomain' + parsed_globals = mock.Mock() + parsed_globals.endpoint_url = None with self.assertRaises(ValueError): - validate_endpoint_url(parsed_args) + validate_endpoint_url(parsed_globals) if __name__ == "__main__":