Skip to content

Commit

Permalink
Don't validate --endpoint-url for help subcommand
Browse files Browse the repository at this point in the history
"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.
  • Loading branch information
jamesls committed Jun 26, 2014
1 parent 9688da1 commit b65f1b8
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 9 deletions.
8 changes: 4 additions & 4 deletions awscli/customizations/cloudsearchdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
2 changes: 1 addition & 1 deletion awscli/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions tests/integration/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 15 additions & 4 deletions tests/unit/customizations/test_cloudsearchdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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__":
Expand Down

0 comments on commit b65f1b8

Please sign in to comment.