Merge pull request #273 from Yelp/return_codes_instead_of_exit

Use returncode instead of sys.exit in the paasta cli

paasta_tools/cli/cli.py

@@ -15,6 +15,7 @@
 # PYTHON_ARGCOMPLETE_OK
 """A command line tool for viewing information from the PaaSTA stack."""
 import argparse
+import sys
 
 import argcomplete
 import pkg_resources
@@ -93,7 +94,8 @@ def main(argv=None):
     """
     configure_log()
     args = parse_args(argv)
-    args.command(args)
+    return_code = args.command(args)
+    sys.exit(return_code)
 
 if __name__ == '__main__':
     main()

paasta_tools/cli/cmds/cook_image.py

@@ -65,7 +65,7 @@ def paasta_cook_image(args, service=None, soa_dir=None):
     if not makefile_responds_to('cook-image'):
         sys.stderr.write('ERROR: local-run now requires a cook-image target to be present in the Makefile. See '
                          'http://paasta.readthedocs.org/en/latest/about/contract.html\n')
-        sys.exit(1)
+        return 1
 
     try:
         cmd = 'make cook-image'
@@ -84,8 +84,8 @@ def paasta_cook_image(args, service=None, soa_dir=None):
                 component='build',
                 level='event',
             )
-            sys.exit(returncode)
+            return returncode
 
     except KeyboardInterrupt:
         sys.stderr.write('\nProcess interrupted by the user. Cancelling.\n')
-        sys.exit(2)
+        return 2

paasta_tools/cli/cmds/generate_pipeline.py

@@ -15,7 +15,6 @@
 """Contains methods used by the paasta client to generate a Jenkins build
 pipeline."""
 import re
-import sys
 
 from paasta_tools.cli.utils import guess_service_name
 from paasta_tools.cli.utils import lazy_choices_completer
@@ -54,7 +53,7 @@ def paasta_generate_pipeline(args):
         validate_service_name(service)
     except NoSuchService as service_not_found:
         print service_not_found
-        sys.exit(1)
+        return 1
 
     generate_pipeline(service=service)
 
@@ -116,4 +115,4 @@ def generate_pipeline(service):
         if returncode != 0:
             print "ERROR: Failed to generate Jenkins pipeline"
             print output
-            sys.exit(returncode)
+            return returncode

paasta_tools/cli/cmds/itest.py

@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import os
-import sys
 
 from paasta_tools.cli.utils import get_jenkins_build_output_url
 from paasta_tools.cli.utils import validate_service_name
@@ -95,4 +94,4 @@ def paasta_itest(args):
             component='build',
             level='event',
         )
-    sys.exit(returncode)
+    return returncode

paasta_tools/cli/cmds/local_run.py

@@ -678,4 +678,4 @@ def paasta_local_run(args):
         )
     except errors.APIError as e:
         sys.stderr.write('Can\'t run Docker container. Error: %s\n' % str(e))
-        sys.exit(1)
+        return 1

paasta_tools/cli/cmds/mark_for_deployment.py

@@ -15,8 +15,6 @@
 """Contains methods used by the paasta client to mark a docker image for
 deployment to a cluster.instance.
 """
-import sys
-
 from paasta_tools import remote_git
 from paasta_tools.cli.utils import validate_service_name
 from paasta_tools.utils import _log
@@ -103,10 +101,9 @@ def paasta_mark_for_deployment(args):
     if service and service.startswith('services-'):
         service = service.split('services-', 1)[1]
     validate_service_name(service)
-    returncode = mark_for_deployment(
+    return mark_for_deployment(
         git_url=args.git_url,
         deploy_group=deploy_group,
         service=service,
         commit=args.commit
     )
-    sys.exit(returncode)

paasta_tools/cli/cmds/push_to_registry.py

@@ -15,8 +15,6 @@
 """Contains methods used by the paasta client to upload a docker
 image to a registry.
 """
-import sys
-
 from paasta_tools.cli.utils import get_jenkins_build_output_url
 from paasta_tools.cli.utils import validate_service_name
 from paasta_tools.utils import _log
@@ -96,4 +94,4 @@ def paasta_push_to_registry(args):
             component='build',
             level='event',
         )
-    sys.exit(returncode)
+    return returncode

paasta_tools/cli/cmds/rollback.py

@@ -12,8 +12,6 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import sys
-
 from service_configuration_lib import DEFAULT_SOA_DIR
 
 from paasta_tools.cli.cmds.mark_for_deployment import mark_for_deployment
@@ -109,4 +107,4 @@ def paasta_rollback(args):
             commit=commit,
         )
 
-    sys.exit(returncode)
+    return returncode

paasta_tools/cli/cmds/security_check.py

@@ -12,7 +12,6 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import sys
 
 
 def add_subparser(subparsers):
@@ -37,4 +36,4 @@ def add_subparser(subparsers):
 
 def perform_security_check(args):
     print 'Not implemented yet'
-    sys.exit(0)
+    return 0

paasta_tools/cli/cmds/start_stop_restart.py

@@ -14,7 +14,6 @@
 # limitations under the License.
 import datetime
 import socket
-import sys
 
 from service_configuration_lib import DEFAULT_SOA_DIR
 
@@ -138,7 +137,7 @@ def paasta_start_or_stop(args, desired_state):
         print "No branches found for %s in %s." % \
             (service_config.get_deploy_group(), remote_refs)
         print "Has it been deployed there yet?"
-        sys.exit(1)
+        return 1
 
     force_bounce = utils.format_timestamp(datetime.datetime.utcnow())
     issue_state_change_for_service(

paasta_tools/cli/cmds/validate.py

@@ -15,7 +15,6 @@
 import json
 import os
 import pkgutil
-import sys
 from glob import glob
 
 import yaml
@@ -264,4 +263,4 @@ def paasta_validate(args):
     service_path = get_service_path(service, soa_dir)
 
     if not paasta_validate_soa_configs(service_path):
-        sys.exit(1)
+        return 1

tests/cli/test_cmds_cook_image.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import mock
-from pytest import raises
 
 from paasta_tools.cli.cmds.cook_image import paasta_cook_image
 
@@ -49,10 +48,7 @@ def test_run_makefile_fail(
     args = mock.MagicMock()
     args.service = 'fake_service'
 
-    with raises(SystemExit) as excinfo:
-        paasta_cook_image(args)
-
-    assert excinfo.value.code == 1
+    assert paasta_cook_image(args) == 1
 
 
 class FakeKeyboardInterrupt(KeyboardInterrupt):
@@ -75,7 +71,4 @@ def test_run_keyboard_interrupt(
     args = mock.MagicMock()
     args.service = 'fake_service'
 
-    with raises(SystemExit) as excinfo:
-        paasta_cook_image(args)
-
-    assert excinfo.value.code == 2
+    assert paasta_cook_image(args) == 2

tests/cli/test_cmds_generate_pipeline.py

@@ -38,12 +38,8 @@ def test_paasta_generate_pipeline_service_not_found(
     args.service = None
     expected_output = "%s\n" % NoSuchService.GUESS_ERROR_MSG
 
-    # Fail if exit(1) does not get called
-    with raises(SystemExit) as sys_exit:
-        paasta_generate_pipeline(args)
-
+    assert paasta_generate_pipeline(args) == 1
     output = mock_stdout.getvalue()
-    assert sys_exit.value.code == 1
     assert output == expected_output
 
 
@@ -57,9 +53,7 @@ def test_generate_pipeline_run_fails(
 ):
     mock_get_team_email_address.return_value = 'fake_email'
     mock_run.return_value = (1, 'Big bad wolf')
-    with raises(SystemExit) as sys_exit:
-        generate_pipeline('fake_service')
-    assert sys_exit.value.code == 1
+    assert generate_pipeline('fake_service') == 1
 
 
 @patch('paasta_tools.cli.cmds.generate_pipeline.get_team_email_address', autospec=True)

tests/cli/test_cmds_itest.py

@@ -19,37 +19,32 @@
 
 @patch('paasta_tools.cli.cmds.itest.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.itest._run', autospec=True)
-@patch('sys.exit')
 @patch('paasta_tools.cli.cmds.itest._log', autospec=True)
 @patch('paasta_tools.cli.cmds.itest.check_docker_image', autospec=True)
 @patch('paasta_tools.cli.cmds.itest.build_docker_tag', autospec=True)
 def test_itest_run_fail(
     mock_build_docker_tag,
     mock_docker_image,
     mock_log,
-    mock_exit,
     mock_run,
     mock_validate_service_name,
 ):
     mock_build_docker_tag.return_value = 'fake-registry/services-foo:paasta-bar'
     mock_docker_image.return_value = True
     mock_run.return_value = (1, 'fake_output')
     args = MagicMock()
-    paasta_itest(args)
-    mock_exit.assert_called_once_with(1)
+    assert paasta_itest(args) == 1
 
 
 @patch('paasta_tools.cli.cmds.itest.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.itest._run', autospec=True)
-@patch('sys.exit')
 @patch('paasta_tools.cli.cmds.itest._log', autospec=True)
 @patch('paasta_tools.cli.cmds.itest.check_docker_image', autospec=True)
 @patch('paasta_tools.cli.cmds.itest.build_docker_tag', autospec=True)
 def test_itest_success(
     mock_build_docker_tag,
     mock_docker_image,
     mock_log,
-    mock_exit,
     mock_run,
     mock_validate_service_name,
 ):
@@ -58,18 +53,16 @@ def test_itest_success(
     mock_run.return_value = (0, 'Yeeehaaa')
 
     args = MagicMock()
-    assert paasta_itest(args) is None
+    assert paasta_itest(args) is 0
 
 
 @patch('paasta_tools.cli.cmds.itest.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.itest._run', autospec=True)
 @patch('paasta_tools.cli.cmds.itest.build_docker_tag', autospec=True)
 @patch('paasta_tools.cli.cmds.itest._log', autospec=True)
-@patch('sys.exit')
 @patch('paasta_tools.cli.cmds.itest.check_docker_image', autospec=True)
 def test_itest_works_when_service_name_starts_with_services_dash(
     mock_docker_image,
-    mock_exit,
     mock_log,
     mock_build_docker_tag,
     mock_run,
@@ -81,5 +74,5 @@ def test_itest_works_when_service_name_starts_with_services_dash(
     args = MagicMock()
     args.service = 'services-fake_service'
     args.commit = 'unused'
-    assert paasta_itest(args) is None
+    assert paasta_itest(args) is 0
     mock_build_docker_tag.assert_called_once_with('fake_service', 'unused')

tests/cli/test_cmds_mark_for_deployment.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 from mock import ANY
 from mock import patch
-from pytest import raises
 
 from paasta_tools.cli.cmds import mark_for_deployment
 
@@ -32,17 +31,14 @@ def test_paasta_mark_for_deployment_acts_like_main(
     mock_validate_service_name,
 ):
     mock_mark_for_deployment.return_value = 42
-    with raises(SystemExit) as sys_exit:
-        mark_for_deployment.paasta_mark_for_deployment(fake_args)
+    assert mark_for_deployment.paasta_mark_for_deployment(fake_args) == 42
     mock_mark_for_deployment.assert_called_once_with(
         service='test_service',
         deploy_group='test_deploy_group',
         commit='fake-hash',
         git_url='git://false.repo/services/test_services',
     )
-
     assert mock_validate_service_name.called
-    assert sys_exit.value.code == 42
 
 
 @patch('paasta_tools.remote_git.create_remote_refs', autospec=True)

tests/cli/test_cmds_push_to_registry.py

@@ -30,9 +30,7 @@ def test_build_command(mock_build_docker_tag):
 @patch('paasta_tools.cli.cmds.push_to_registry.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._run', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._log', autospec=True)
-@patch('sys.exit', autospec=True)
 def test_push_to_registry_run_fail(
-    mock_exit,
     mock_log,
     mock_run,
     mock_validate_service_name,
@@ -41,17 +39,14 @@ def test_push_to_registry_run_fail(
     mock_build_command.return_value = 'docker push my-docker-registry/services-foo:paasta-asdf'
     mock_run.return_value = (1, 'Bad')
     args = MagicMock()
-    paasta_push_to_registry(args)
-    mock_exit.assert_called_once_with(1)
+    assert paasta_push_to_registry(args) == 1
 
 
 @patch('paasta_tools.cli.cmds.push_to_registry.build_command')
 @patch('paasta_tools.cli.cmds.push_to_registry.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._run', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._log', autospec=True)
-@patch('sys.exit', autospec=True)
 def test_push_to_registry_success(
-    mock_exit,
     mock_log,
     mock_run,
     mock_validate_service_name,
@@ -60,16 +55,14 @@ def test_push_to_registry_success(
     mock_build_command.return_value = 'docker push my-docker-registry/services-foo:paasta-asdf'
     mock_run.return_value = (0, 'Success')
     args = MagicMock()
-    assert paasta_push_to_registry(args) is None
+    assert paasta_push_to_registry(args) == 0
 
 
 @patch('paasta_tools.cli.cmds.push_to_registry.validate_service_name', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._run', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry._log', autospec=True)
 @patch('paasta_tools.cli.cmds.push_to_registry.build_command', autospec=True)
-@patch('sys.exit', autospec=True)
 def test_push_to_registry_works_when_service_name_starts_with_services_dash(
-    mock_exit,
     mock_build_command,
     mock_log,
     mock_run,
@@ -79,5 +72,5 @@ def test_push_to_registry_works_when_service_name_starts_with_services_dash(
     args = MagicMock()
     args.service = 'fake_service'
     args.commit = 'unused'
-    assert paasta_push_to_registry(args) is None
+    assert paasta_push_to_registry(args) == 0
     mock_build_command.assert_called_once_with('fake_service', 'unused')