From 929de38baa76f54aea13b915de7b90944fed2061 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 10 Feb 2017 14:00:40 +0100 Subject: [PATCH 01/13] Fix st2 login command - ConfigParser object doesn't behave like a dict. --- st2client/st2client/commands/auth.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py index 93194d1e2c..7c7e39fc76 100644 --- a/st2client/st2client/commands/auth.py +++ b/st2client/st2client/commands/auth.py @@ -123,17 +123,17 @@ def run(self, args, **kwargs): config.read(config_file) # Modify config (and optionally populate with password) - if 'credentials' not in config: + if not config.has_section('credentials'): config.add_section('credentials') - config['credentials'] = {} - config['credentials']['username'] = args.username + + config.set('credentials', 'username', args.username) if args.write_password: - config['credentials']['password'] = args.password + config.set('credentials', 'password', args.password) else: # Remove any existing password from config - config['credentials'].pop('password', None) + config.remove_option('credentials', 'password') - with open(config_file, "w") as cfg_file_out: + with open(config_file, 'w') as cfg_file_out: config.write(cfg_file_out) return manager From c8e08f643c1bc80099bb869a6b7e9b2d0f14f012 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 10 Feb 2017 14:12:30 +0100 Subject: [PATCH 02/13] Also fix whoami command. --- st2client/st2client/commands/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py index 7c7e39fc76..c301cd5048 100644 --- a/st2client/st2client/commands/auth.py +++ b/st2client/st2client/commands/auth.py @@ -175,7 +175,7 @@ def run(self, args, **kwargs): config = ConfigParser() config.read(config_file) - return config['credentials']['username'] + return config.get('credentials', 'username') def run_and_print(self, args, **kwargs): try: From 4a9be54ff35c16b03b0399af92af6425cf747ad9 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 10 Feb 2017 14:28:44 +0100 Subject: [PATCH 03/13] Use ConfigParser from six.moves everywhere. --- st2client/st2client/commands/auth.py | 3 ++- st2debug/st2debug/processors.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/st2client/st2client/commands/auth.py b/st2client/st2client/commands/auth.py index c301cd5048..c37ba6955a 100644 --- a/st2client/st2client/commands/auth.py +++ b/st2client/st2client/commands/auth.py @@ -13,11 +13,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from six.moves.configparser import ConfigParser import getpass import json import logging +from six.moves.configparser import ConfigParser + from st2client.base import BaseCLIApp from st2client import config_parser from st2client import models diff --git a/st2debug/st2debug/processors.py b/st2debug/st2debug/processors.py index 12fb9029f9..711b3ae5b8 100644 --- a/st2debug/st2debug/processors.py +++ b/st2debug/st2debug/processors.py @@ -15,7 +15,7 @@ import os -from ConfigParser import ConfigParser +from six.moves.configparser import ConfigParser __all__ = [ 'process_st2_config', From ea4c1212d64beadb720a5339c59db84c9f922e7c Mon Sep 17 00:00:00 2001 From: Matt Oswalt Date: Fri, 10 Feb 2017 13:48:36 -0800 Subject: [PATCH 04/13] Total re-vamp of login and whoami tests The previous version of these tests were a bit too reliant on mock, so this approach instead works with as few mocks as possible. Naturally, we have to keep the mock for the st2 API interactions, but these tests run the `st2 login` and `st2 whoami` commands using real config files, as opposed to a mocked ConfigParser. Signed-off-by: Matt Oswalt --- st2client/tests/unit/test_auth.py | 439 ++++++++++++++++-------------- 1 file changed, 240 insertions(+), 199 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index a35cede30a..0c30b0bfa6 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from mock import call import os import uuid import json @@ -22,9 +21,10 @@ import requests import argparse import logging +from cStringIO import StringIO +import sys from tests import base -from st2client import config_parser from st2client import shell from st2client.models.core import add_auth_token_to_kwargs_from_env from st2client.commands.resource import add_auth_token_to_kwargs_from_cli @@ -41,109 +41,92 @@ } -class TestWhoami(base.BaseCLITestCase): +class CaptureStdout(list): + """This is a bit of borrowed code to make it easier to read from stdout + + It's meant to be used within a context manager. + """ + + def __enter__(self): + self._stdout = sys.stdout + sys.stdout = self._stringio = StringIO() + return self + + def __exit__(self, *args): + self.extend(self._stringio.getvalue().splitlines()) + del self._stringio # free up some memory + sys.stdout = self._stdout + + +class TestLoginBase(base.BaseCLITestCase): + '''A base class for testing related to 'st2 login' commands + + This exists primarily to ensure that each specific test case is kept atomic, + since the tests create actual files on the filesystem - as well as to cut down + on duplicate code in each test class + ''' + + DOTST2_PATH = os.path.expanduser('~/.st2/') def __init__(self, *args, **kwargs): - super(TestWhoami, self).__init__(*args, **kwargs) + self.config_file = kwargs.pop('config_file', '~/.st2/config') + self.config_contents = kwargs.pop('config_contents', None) + super(TestLoginBase, self).__init__(*args, **kwargs) + self.parser = argparse.ArgumentParser() self.parser.add_argument('-t', '--token', dest='token') self.parser.add_argument('--api-key', dest='api_key') self.shell = shell.Shell() - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_whoami(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test 'st2 whoami' functionality - """ + def setUp(self): + super(TestLoginBase, self).setUp() - # Mock config - config_file = config_parser.ST2_CONFIG_PATH - self.shell._get_config_file_path = mock.MagicMock(return_value="/tmp/st2config") - mock_cli.return_value._get_config_file_path.return_value = config_file + # Remove any existing config file + if os.path.isfile(self.config_file): + os.remove(self.config_file) - self.shell.run(['whoami']) + with open(self.config_file, 'w') as cfg: - mock_cfg.return_value.__getitem__.assert_called_with('credentials') - mock_cfg.return_value.__getitem__('credentials').__getitem__.assert_called_with('username') + # If a test passes in it's own config, we write that instead + if self.config_contents: + for line in self.config_contents.split('\n'): + cfg.write("%s\n" % line.strip()) + else: - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_whoami_not_logged_in(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test 'st2 whoami' functionality with a missing username - """ - - # Mock config - config_file = config_parser.ST2_CONFIG_PATH - self.shell._get_config_file_path = mock.MagicMock(return_value="/tmp/st2config") - mock_cli.return_value._get_config_file_path.return_value = config_file - - # Trigger keyerror exception when trying to access username. - # We have to do it this way because ConfigParser acts like a - # dict but also has methods like read() - attrs = {'__getitem__.side_effect': KeyError} - mock_cfg.return_value.__getitem__.return_value.configure_mock(**attrs) - - # assert that the config field lookup caused the CLI to return an error code - # we are also using "--debug" flag to ensure the exception is re-raised once caught - self.assertEqual( - self.shell.run(['--debug', 'whoami']), - 1 - ) + # Default config for most tests + cfg.write('[credentials]\n') + # Using 'olduser' so we can assert this has changed at the end + cfg.write('username = olduser\n') + cfg.write('password = Password1!\n') - # Some additional asserts to ensure things are being called correctly - mock_cfg.return_value.__getitem__.assert_called_with('credentials') - mock_cfg.return_value.__getitem__.return_value.__getitem__.assert_called_with('username') + def tearDown(self): + super(TestLoginBase, self).tearDown() + + # Clean up config file + os.remove(self.config_file) + + # Clean up tokens + for file in [f for f in os.listdir(self.DOTST2_PATH) if 'token-' in f]: + os.remove(self.DOTST2_PATH + file) - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_whoami_missing_credentials_section(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test 'st2 whoami' functionality with a missing credentials section - """ - - # mocked config that is empty (no credentials section at all) - mock_cfg.return_value.read.return_value = {} - - # Mock config - config_file = config_parser.ST2_CONFIG_PATH - self.shell._get_config_file_path = mock.MagicMock(return_value="/tmp/st2config") - mock_cli.return_value._get_config_file_path.return_value = config_file - - # Trigger keyerror exception when trying to access username. - # We have to do it this way because ConfigParser acts like a - # dict but also has methods like read() - attrs = {'__getitem__.side_effect': KeyError} - mock_cfg.return_value.configure_mock(**attrs) - - # assert that the config field lookup caused the CLI to return an error code - # we are also using "--debug" flag to ensure the exception is re-raised once caught - self.assertEqual( - self.shell.run(['--debug', 'whoami']), - 1 - ) - # An additional assert to ensure things are being called correctly - mock_cfg.return_value.__getitem__.assert_called_with('credentials') +class TestWhoami(TestLoginBase): + CONFIG_FILE = '/tmp/logintest.cfg' -class TestLogin(base.BaseCLITestCase): + USERNAME = 'st2foouser' def __init__(self, *args, **kwargs): - super(TestLogin, self).__init__(*args, **kwargs) + + new_config = (""" + [credentials] + username = %s + password = Password1! + """ % self.USERNAME) + + super(TestWhoami, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) self.parser = argparse.ArgumentParser() self.parser.add_argument('-t', '--token', dest='token') self.parser.add_argument('--api-key', dest='api_key') @@ -152,167 +135,225 @@ def __init__(self, *args, **kwargs): @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_login_password_and_config(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test the 'st2 login' functionality by providing a password, and specifying a configuration file - """ - - args = ['--config', '/tmp/st2config', 'login', 'st2admin', '--password', 'Password1!'] - expected_username = 'st2admin' + @mock.patch('st2client.commands.auth.print') + def test_whoami(self, mock_print): + '''Test 'st2 whoami' functionality + ''' - mock_gp.getpass.return_value = "Password1!" + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - # Mock config - config_file = args[args.index('--config') + 1] - self.shell._get_config_file_path = mock.MagicMock(return_value="/tmp/st2config") - mock_cli.return_value._get_config_file_path.return_value = config_file + self.assertEqual(retcode, 0) + self.assertTrue(self.USERNAME in output[0]) - self.shell.run(args) - # Ensure getpass was only used if "--password" option was omitted - mock_gp.getpass.assert_not_called() +class TestWhoamiMissingUser(TestLoginBase): - # Ensure token was generated - mock_cli.return_value._cache_auth_token.assert_called_once() + CONFIG_FILE = '/tmp/logintest.cfg' - # Build list of expected calls for mock_cfg - config_calls = [call('username', expected_username)] + def __init__(self, *args, **kwargs): - # Ensure that the password field was removed from the config - mock_cfg.return_value.__getitem__.return_value.pop.assert_called_once_with('password', None) + new_config = (""" + [credentials] + foo = bar + """) - # Run common assertions for testing login functionality - self._login_common_assertions(mock_gp, mock_cli, mock_open, mock_cfg, - config_calls, config_file) + super(TestWhoamiMissingUser, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_login_no_password(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test the 'st2 login' functionality without the "--password" arg - """ + @mock.patch('st2client.commands.auth.print') + def test_whoami(self, mock_print): + '''Test 'st2 whoami' functionality with a missing username + ''' - args = ['login', 'st2admin'] - expected_username = 'st2admin' + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - mock_gp.getpass.return_value = "Password1!" + self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + self.assertEqual(retcode, 0) - config_file = config_parser.ST2_CONFIG_PATH - mock_cli.return_value._get_config_file_path.return_value = config_file - self.shell.run(args) +class TestWhoamiMissingCreds(TestLoginBase): - # Ensure getpass was only used if "--password" option was omitted - mock_gp.getpass.assert_called_once() + CONFIG_FILE = '/tmp/logintest.cfg' - # Ensure token was generated - mock_cli.return_value._cache_auth_token.assert_called_once() - - config_calls = [call('username', expected_username)] + def __init__(self, *args, **kwargs): - # Ensure that the password field was removed from the config - mock_cfg.return_value.__getitem__.return_value.pop.assert_called_once_with('password', None) + new_config = (""" + [nonsense] + foo = bar + """) - # Run common assertions for testing login functionality - self._login_common_assertions(mock_gp, mock_cli, mock_open, mock_cfg, - config_calls, config_file) + super(TestWhoamiMissingCreds, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_login_password_with_write(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test the 'st2 login' functionality by providing a password, and writing it to config file - """ + @mock.patch('st2client.commands.auth.print') + def test_whoami(self, mock_print): + '''Test 'st2 whoami' functionality with a missing credentials section + ''' + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + self.assertEqual(retcode, 0) - args = ['login', 'st2admin', '--password', 'Password1!', '-w'] - expected_username = 'st2admin' - mock_gp.getpass.return_value = "Password1!" +class TestLoginPasswordAndConfig(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + TOKEN = { + 'user': 'st2admin', + 'token': '44583f15945b4095afbf57058535ca64', + 'expiry': '2017-02-12T00:53:09.632783Z', + 'id': '589e607532ed3535707f10eb', + 'metadata': {} + } + + def __init__(self, *args, **kwargs): + super(TestLoginPasswordAndConfig, self).__init__( + config_file=self.CONFIG_FILE, *args, **kwargs + ) + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) + def runTest(self): + '''Test 'st2 login' functionality by specifying a password and a configuration file + ''' - config_file = config_parser.ST2_CONFIG_PATH - mock_cli.return_value._get_config_file_path.return_value = config_file + expected_username = self.TOKEN['user'] + args = ['--config', self.CONFIG_FILE, 'login', expected_username, '--password', + 'Password1!'] self.shell.run(args) - # Ensure getpass was only used if "--password" option was omitted - mock_gp.getpass.assert_not_called() + with open(self.CONFIG_FILE, 'r') as config_file: + + for line in config_file.readlines(): + + # Make sure certain values are not present + self.assertFalse('password' in line) + self.assertFalse('olduser' in line) - # Ensure token was generated - mock_cli.return_value._cache_auth_token.assert_called_once() + # Make sure configured username is what we expect + if 'username' in line: + self.assertEquals(line.split(' ')[2][:-1], expected_username) - # Build list of expected calls for mock_cfg - config_calls = [call('username', expected_username), call('password', 'Password1!')] + # validate token was created + self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username))) - # Run common assertions for testing login functionality - self._login_common_assertions(mock_gp, mock_cli, mock_open, mock_cfg, - config_calls, config_file) + +class TestLoginIntPwdAndConfig(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + TOKEN = { + 'user': 'st2admin', + 'token': '44583f15945b4095afbf57058535ca64', + 'expiry': '2017-02-12T00:53:09.632783Z', + 'id': '589e607532ed3535707f10eb', + 'metadata': {} + } + + def __init__(self, *args, **kwargs): + super(TestLoginIntPwdAndConfig, self).__init__( + config_file=self.CONFIG_FILE, *args, **kwargs + ) @mock.patch.object( requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch("st2client.commands.auth.ConfigParser") - @mock.patch("st2client.commands.auth.open") - @mock.patch("st2client.commands.auth.BaseCLIApp") - @mock.patch("st2client.commands.auth.getpass") - def test_login_no_password_with_write_and_config(self, mock_gp, mock_cli, mock_open, mock_cfg): - """Test the 'st2 login' functionality by providing config file and writing password to it - """ - - args = ['--config', '/tmp/st2config', 'login', 'st2admin', '-w'] - expected_username = 'st2admin' + mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) + @mock.patch('st2client.commands.auth.getpass') + def runTest(self, mock_gp): + '''Test 'st2 login' functionality with interactive password entry + ''' - mock_gp.getpass.return_value = "Password1!" + expected_username = self.TOKEN['user'] + args = ['--config', self.CONFIG_FILE, 'login', expected_username] - # Mock config - config_file = args[args.index('--config') + 1] - self.shell._get_config_file_path = mock.MagicMock(return_value="/tmp/st2config") - mock_cli.return_value._get_config_file_path.return_value = config_file + mock_gp.getpass.return_value = 'Password1!' self.shell.run(args) - mock_gp.getpass.assert_called_once() + with open(self.CONFIG_FILE, 'r') as config_file: - # Ensure token was generated - mock_cli.return_value._cache_auth_token.assert_called_once() + for line in config_file.readlines(): - # Build list of expected calls for mock_cfg - config_calls = [call('username', expected_username), call('password', 'Password1!')] + # Make sure certain values are not present + self.assertFalse('password' in line) + self.assertFalse('olduser' in line) - # Run common assertions for testing login functionality - self._login_common_assertions(mock_gp, mock_cli, mock_open, mock_cfg, - config_calls, config_file) + # Make sure configured username is what we expect + if 'username' in line: + self.assertEquals(line.split(' ')[2][:-1], expected_username) - def _login_common_assertions(self, mock_gp, mock_cli, mock_open, mock_cfg, - config_calls, config_file): - # Ensure file was written to with a context manager - mock_open.return_value.__enter__.assert_called_once() - mock_open.return_value.__exit__.assert_called_once() + # validate token was created + self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username))) - # Make sure 'credentials' key in config was initialized properly - mock_cfg.return_value.__setitem__.assert_has_calls( - [call('credentials', {})], any_order=True - ) - # Ensure configuration was performed properly - mock_open.assert_called_once_with(config_file, 'w') - mock_cfg.return_value.read.assert_called_once_with(config_file) - mock_cfg.return_value.add_section.assert_called_once_with('credentials') - mock_cfg.return_value.__getitem__.return_value.__setitem__.assert_has_calls( - config_calls, - any_order=True +class TestLoginPwdOkay(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + TOKEN = { + 'user': 'st2admin', + 'token': '44583f15945b4095afbf57058535ca64', + 'expiry': '2017-02-12T00:53:09.632783Z', + 'id': '589e607532ed3535707f10eb', + 'metadata': {} + } + + def __init__(self, *args, **kwargs): + super(TestLoginPwdOkay, self).__init__( + config_file=self.CONFIG_FILE, *args, **kwargs ) + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) + @mock.patch('st2client.commands.auth.getpass') + def runTest(self, mock_gp): + '''Test 'st2 login' functionality with --write-password flag set + ''' + + expected_username = self.TOKEN['user'] + args = ['--config', self.CONFIG_FILE, 'login', expected_username, '--password', + 'Password1!', '--write-password'] + + self.shell.run(args) + + with open(self.CONFIG_FILE, 'r') as config_file: + + for line in config_file.readlines(): + + # Make sure certain values are not present + self.assertFalse('olduser' in line) + + # Make sure configured username is what we expect + if 'username' in line: + self.assertEquals(line.split(' ')[2][:-1], expected_username) + + # validate token was created + self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username))) + class TestAuthToken(base.BaseCLITestCase): From ccaf339d3f2f4a303c40ae5b39e4f85f7ea8390d Mon Sep 17 00:00:00 2001 From: Matt Oswalt Date: Sun, 12 Feb 2017 16:20:27 -0800 Subject: [PATCH 05/13] Added a few more tests; removed refs to 'print' mock experiment --- st2client/tests/unit/test_auth.py | 91 ++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index 0c30b0bfa6..7527e67d42 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -135,8 +135,7 @@ def __init__(self, *args, **kwargs): @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch('st2client.commands.auth.print') - def test_whoami(self, mock_print): + def test_whoami(self): '''Test 'st2 whoami' functionality ''' @@ -169,8 +168,7 @@ def __init__(self, *args, **kwargs): @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch('st2client.commands.auth.print') - def test_whoami(self, mock_print): + def test_whoami(self): '''Test 'st2 whoami' functionality with a missing username ''' @@ -203,8 +201,7 @@ def __init__(self, *args, **kwargs): @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch('st2client.commands.auth.print') - def test_whoami(self, mock_print): + def test_whoami(self): '''Test 'st2 whoami' functionality with a missing credentials section ''' @@ -215,6 +212,46 @@ def test_whoami(self, mock_print): self.assertEqual(retcode, 0) +class TestWhoamiUncaughtException(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + USERNAME = 'st2foouser' + + def __init__(self, *args, **kwargs): + + new_config = (""" + [credentials] + username = %s + password = Password1! + """ % self.USERNAME) + + super(TestWhoamiUncaughtException, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + @mock.patch('st2client.commands.auth.BaseCLIApp') + def test_whoami(self, mock_cli): + '''Test 'st2 whoami' ability to detect unhandled exceptions + ''' + + # Only mocking "BaseCLIApp" here in order to generate an exception for this specific test + mock_cli.return_value._get_config_file_path = mock.MagicMock(side_effect=Exception) + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual("Unable to retrieve currently logged-in user", output[0]) + self.assertEqual(retcode, 0) + + class TestLoginPasswordAndConfig(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' @@ -309,7 +346,7 @@ def runTest(self, mock_gp): self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username))) -class TestLoginPwdOkay(TestLoginBase): +class TestLoginWritePwdOkay(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' @@ -322,7 +359,7 @@ class TestLoginPwdOkay(TestLoginBase): } def __init__(self, *args, **kwargs): - super(TestLoginPwdOkay, self).__init__( + super(TestLoginWritePwdOkay, self).__init__( config_file=self.CONFIG_FILE, *args, **kwargs ) @@ -355,6 +392,44 @@ def runTest(self, mock_gp): self.assertTrue(os.path.isfile('%stoken-%s' % (self.DOTST2_PATH, expected_username))) +class TestLoginUncaughtException(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + TOKEN = { + 'user': 'st2admin', + 'token': '44583f15945b4095afbf57058535ca64', + 'expiry': '2017-02-12T00:53:09.632783Z', + 'id': '589e607532ed3535707f10eb', + 'metadata': {} + } + + def __init__(self, *args, **kwargs): + super(TestLoginUncaughtException, self).__init__( + config_file=self.CONFIG_FILE, *args, **kwargs + ) + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) + @mock.patch('st2client.commands.auth.getpass') + def runTest(self, mock_gp): + '''Test 'st2 login' ability to detect unhandled exceptions + ''' + + expected_username = self.TOKEN['user'] + args = ['--config', self.CONFIG_FILE, 'login', expected_username] + + mock_gp.getpass = mock.MagicMock(side_effect=Exception) + + self.shell.run(args) + with CaptureStdout() as output: + retcode = self.shell.run(args) + + self.assertTrue("Failed to log in as %s" % expected_username in output[0]) + self.assertEqual(retcode, 0) + + class TestAuthToken(base.BaseCLITestCase): def __init__(self, *args, **kwargs): From 50c8dc0ee8855a8d2b06fe1303caa5512c118146 Mon Sep 17 00:00:00 2001 From: Matt Oswalt Date: Sun, 12 Feb 2017 16:21:33 -0800 Subject: [PATCH 06/13] Reordered test_auth tests to reflect the order of functions being tested --- st2client/tests/unit/test_auth.py | 284 +++++++++++++++--------------- 1 file changed, 142 insertions(+), 142 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index 7527e67d42..c72d54673c 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -110,148 +110,6 @@ def tearDown(self): os.remove(self.DOTST2_PATH + file) -class TestWhoami(TestLoginBase): - - CONFIG_FILE = '/tmp/logintest.cfg' - - USERNAME = 'st2foouser' - - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - username = %s - password = Password1! - """ % self.USERNAME) - - super(TestWhoami, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() - - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): - '''Test 'st2 whoami' functionality - ''' - - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - - self.assertEqual(retcode, 0) - self.assertTrue(self.USERNAME in output[0]) - - -class TestWhoamiMissingUser(TestLoginBase): - - CONFIG_FILE = '/tmp/logintest.cfg' - - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - foo = bar - """) - - super(TestWhoamiMissingUser, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() - - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): - '''Test 'st2 whoami' functionality with a missing username - ''' - - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - - self.assertEqual('Unable to retrieve currently logged-in user', output[0]) - self.assertEqual(retcode, 0) - - -class TestWhoamiMissingCreds(TestLoginBase): - - CONFIG_FILE = '/tmp/logintest.cfg' - - def __init__(self, *args, **kwargs): - - new_config = (""" - [nonsense] - foo = bar - """) - - super(TestWhoamiMissingCreds, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() - - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): - '''Test 'st2 whoami' functionality with a missing credentials section - ''' - - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - - self.assertEqual('Unable to retrieve currently logged-in user', output[0]) - self.assertEqual(retcode, 0) - - -class TestWhoamiUncaughtException(TestLoginBase): - - CONFIG_FILE = '/tmp/logintest.cfg' - - USERNAME = 'st2foouser' - - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - username = %s - password = Password1! - """ % self.USERNAME) - - super(TestWhoamiUncaughtException, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() - - @mock.patch.object( - requests, 'post', - mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - @mock.patch('st2client.commands.auth.BaseCLIApp') - def test_whoami(self, mock_cli): - '''Test 'st2 whoami' ability to detect unhandled exceptions - ''' - - # Only mocking "BaseCLIApp" here in order to generate an exception for this specific test - mock_cli.return_value._get_config_file_path = mock.MagicMock(side_effect=Exception) - - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - - self.assertEqual("Unable to retrieve currently logged-in user", output[0]) - self.assertEqual(retcode, 0) - - class TestLoginPasswordAndConfig(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' @@ -430,6 +288,148 @@ def runTest(self, mock_gp): self.assertEqual(retcode, 0) +class TestWhoami(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + USERNAME = 'st2foouser' + + def __init__(self, *args, **kwargs): + + new_config = (""" + [credentials] + username = %s + password = Password1! + """ % self.USERNAME) + + super(TestWhoami, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + def test_whoami(self): + '''Test 'st2 whoami' functionality + ''' + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual(retcode, 0) + self.assertTrue(self.USERNAME in output[0]) + + +class TestWhoamiMissingUser(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + def __init__(self, *args, **kwargs): + + new_config = (""" + [credentials] + foo = bar + """) + + super(TestWhoamiMissingUser, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + def test_whoami(self): + '''Test 'st2 whoami' functionality with a missing username + ''' + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + self.assertEqual(retcode, 0) + + +class TestWhoamiMissingCreds(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + def __init__(self, *args, **kwargs): + + new_config = (""" + [nonsense] + foo = bar + """) + + super(TestWhoamiMissingCreds, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + def test_whoami(self): + '''Test 'st2 whoami' functionality with a missing credentials section + ''' + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + self.assertEqual(retcode, 0) + + +class TestWhoamiUncaughtException(TestLoginBase): + + CONFIG_FILE = '/tmp/logintest.cfg' + + USERNAME = 'st2foouser' + + def __init__(self, *args, **kwargs): + + new_config = (""" + [credentials] + username = %s + password = Password1! + """ % self.USERNAME) + + super(TestWhoamiUncaughtException, self).__init__( + config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs + ) + self.parser = argparse.ArgumentParser() + self.parser.add_argument('-t', '--token', dest='token') + self.parser.add_argument('--api-key', dest='api_key') + self.shell = shell.Shell() + + @mock.patch.object( + requests, 'post', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + @mock.patch('st2client.commands.auth.BaseCLIApp') + def test_whoami(self, mock_cli): + '''Test 'st2 whoami' ability to detect unhandled exceptions + ''' + + # Only mocking "BaseCLIApp" here in order to generate an exception for this specific test + mock_cli.return_value._get_config_file_path = mock.MagicMock(side_effect=Exception) + + with CaptureStdout() as output: + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + + self.assertEqual("Unable to retrieve currently logged-in user", output[0]) + self.assertEqual(retcode, 0) + + class TestAuthToken(base.BaseCLITestCase): def __init__(self, *args, **kwargs): From 41090ab3719beb8f564729980dd5586323e88eec Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 09:41:37 +0100 Subject: [PATCH 07/13] Use StringIO from six. --- st2client/tests/unit/test_auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index c72d54673c..023f3a1d00 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys import os import uuid import json @@ -21,8 +22,8 @@ import requests import argparse import logging -from cStringIO import StringIO -import sys + +from six import StringIO from tests import base from st2client import shell From c1923f64dbae13f318a6aab5aa9dc1eac71ba754 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 09:59:10 +0100 Subject: [PATCH 08/13] Reduce code duplication and increase re-use. --- st2client/tests/unit/test_auth.py | 111 +++++++++++------------------- 1 file changed, 42 insertions(+), 69 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index 023f3a1d00..169474e90f 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -60,19 +60,21 @@ def __exit__(self, *args): class TestLoginBase(base.BaseCLITestCase): - '''A base class for testing related to 'st2 login' commands + """ + A base class for testing related to 'st2 login' commands This exists primarily to ensure that each specific test case is kept atomic, since the tests create actual files on the filesystem - as well as to cut down on duplicate code in each test class - ''' + """ DOTST2_PATH = os.path.expanduser('~/.st2/') - def __init__(self, *args, **kwargs): - self.config_file = kwargs.pop('config_file', '~/.st2/config') - self.config_contents = kwargs.pop('config_contents', None) - super(TestLoginBase, self).__init__(*args, **kwargs) + def __init__(self, config_file='~/.st2/config', config_contents=None): + super(TestLoginBase, self).__init__() + + self.config_file = config_file + self.config_contents = config_contents self.parser = argparse.ArgumentParser() self.parser.add_argument('-t', '--token', dest='token') @@ -87,7 +89,6 @@ def setUp(self): os.remove(self.config_file) with open(self.config_file, 'w') as cfg: - # If a test passes in it's own config, we write that instead if self.config_contents: for line in self.config_contents.split('\n'): @@ -125,8 +126,7 @@ class TestLoginPasswordAndConfig(TestLoginBase): def __init__(self, *args, **kwargs): super(TestLoginPasswordAndConfig, self).__init__( - config_file=self.CONFIG_FILE, *args, **kwargs - ) + config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', @@ -171,8 +171,7 @@ class TestLoginIntPwdAndConfig(TestLoginBase): def __init__(self, *args, **kwargs): super(TestLoginIntPwdAndConfig, self).__init__( - config_file=self.CONFIG_FILE, *args, **kwargs - ) + config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', @@ -219,8 +218,7 @@ class TestLoginWritePwdOkay(TestLoginBase): def __init__(self, *args, **kwargs): super(TestLoginWritePwdOkay, self).__init__( - config_file=self.CONFIG_FILE, *args, **kwargs - ) + config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', @@ -265,8 +263,7 @@ class TestLoginUncaughtException(TestLoginBase): def __init__(self, *args, **kwargs): super(TestLoginUncaughtException, self).__init__( - config_file=self.CONFIG_FILE, *args, **kwargs - ) + config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', @@ -295,26 +292,20 @@ class TestWhoami(TestLoginBase): USERNAME = 'st2foouser' - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - username = %s - password = Password1! - """ % self.USERNAME) + CONFIG_CONTENT = """ + [credentials] + username = %s + password = Password1! + """ % USERNAME + def __init__(self, *args, **kwargs): super(TestWhoami, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() + config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): + def runTest(self): '''Test 'st2 whoami' functionality ''' @@ -329,25 +320,19 @@ class TestWhoamiMissingUser(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - foo = bar - """) + CONFIG_CONTENT = (""" + [credentials] + foo = bar + """) + def __init__(self, *args, **kwargs): super(TestWhoamiMissingUser, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() + config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): + def runTest(self): '''Test 'st2 whoami' functionality with a missing username ''' @@ -362,25 +347,19 @@ class TestWhoamiMissingCreds(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' - def __init__(self, *args, **kwargs): - - new_config = (""" - [nonsense] - foo = bar - """) + CONFIG_CONTENT = (""" + [nonsense] + foo = bar + """) + def __init__(self, *args, **kwargs): super(TestWhoamiMissingCreds, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() + config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) - def test_whoami(self): + def runTest(self): '''Test 'st2 whoami' functionality with a missing credentials section ''' @@ -397,27 +376,21 @@ class TestWhoamiUncaughtException(TestLoginBase): USERNAME = 'st2foouser' - def __init__(self, *args, **kwargs): - - new_config = (""" - [credentials] - username = %s - password = Password1! - """ % self.USERNAME) + CONFIG_CONTENT = (""" + [credentials] + username = %s + password = Password1! + """ % USERNAME) + def __init__(self, *args, **kwargs): super(TestWhoamiUncaughtException, self).__init__( - config_contents=new_config, config_file=self.CONFIG_FILE, *args, **kwargs - ) - self.parser = argparse.ArgumentParser() - self.parser.add_argument('-t', '--token', dest='token') - self.parser.add_argument('--api-key', dest='api_key') - self.shell = shell.Shell() + config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) @mock.patch('st2client.commands.auth.BaseCLIApp') - def test_whoami(self, mock_cli): + def runTest(self, mock_cli): '''Test 'st2 whoami' ability to detect unhandled exceptions ''' From ba85739bbb964ceb01653670f0cd7319ee9b5660 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 10:03:17 +0100 Subject: [PATCH 09/13] Reduce code duplication even further. --- st2client/tests/unit/test_auth.py | 62 +++++++------------------------ 1 file changed, 14 insertions(+), 48 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index 169474e90f..a4069474e0 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -69,12 +69,11 @@ class TestLoginBase(base.BaseCLITestCase): """ DOTST2_PATH = os.path.expanduser('~/.st2/') + CONFIG_FILE = '/tmp/logintest.cfg' + CONFIG_CONTENTS = None - def __init__(self, config_file='~/.st2/config', config_contents=None): - super(TestLoginBase, self).__init__() - - self.config_file = config_file - self.config_contents = config_contents + def __init__(self, *args, **kwargs): + super(TestLoginBase, self).__init__(*args, **kwargs) self.parser = argparse.ArgumentParser() self.parser.add_argument('-t', '--token', dest='token') @@ -85,13 +84,13 @@ def setUp(self): super(TestLoginBase, self).setUp() # Remove any existing config file - if os.path.isfile(self.config_file): - os.remove(self.config_file) + if os.path.isfile(self.CONFIG_FILE): + os.remove(self.CONFIG_FILE) - with open(self.config_file, 'w') as cfg: + with open(self.CONFIG_FILE, 'w') as cfg: # If a test passes in it's own config, we write that instead - if self.config_contents: - for line in self.config_contents.split('\n'): + if self.CONFIG_CONTENTS: + for line in self.CONFIG_CONTENTS.split('\n'): cfg.write("%s\n" % line.strip()) else: @@ -105,7 +104,7 @@ def tearDown(self): super(TestLoginBase, self).tearDown() # Clean up config file - os.remove(self.config_file) + os.remove(self.CONFIG_FILE) # Clean up tokens for file in [f for f in os.listdir(self.DOTST2_PATH) if 'token-' in f]: @@ -124,10 +123,6 @@ class TestLoginPasswordAndConfig(TestLoginBase): 'metadata': {} } - def __init__(self, *args, **kwargs): - super(TestLoginPasswordAndConfig, self).__init__( - config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) @@ -142,7 +137,6 @@ def runTest(self): self.shell.run(args) with open(self.CONFIG_FILE, 'r') as config_file: - for line in config_file.readlines(): # Make sure certain values are not present @@ -169,10 +163,6 @@ class TestLoginIntPwdAndConfig(TestLoginBase): 'metadata': {} } - def __init__(self, *args, **kwargs): - super(TestLoginIntPwdAndConfig, self).__init__( - config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) @@ -216,10 +206,6 @@ class TestLoginWritePwdOkay(TestLoginBase): 'metadata': {} } - def __init__(self, *args, **kwargs): - super(TestLoginWritePwdOkay, self).__init__( - config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) @@ -261,10 +247,6 @@ class TestLoginUncaughtException(TestLoginBase): 'metadata': {} } - def __init__(self, *args, **kwargs): - super(TestLoginUncaughtException, self).__init__( - config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps(TOKEN), 200, 'OK'))) @@ -292,16 +274,12 @@ class TestWhoami(TestLoginBase): USERNAME = 'st2foouser' - CONFIG_CONTENT = """ + CONFIG_CONTENTS = """ [credentials] username = %s password = Password1! """ % USERNAME - def __init__(self, *args, **kwargs): - super(TestWhoami, self).__init__( - config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) @@ -320,15 +298,11 @@ class TestWhoamiMissingUser(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' - CONFIG_CONTENT = (""" + CONFIG_CONTENTS = (""" [credentials] foo = bar """) - def __init__(self, *args, **kwargs): - super(TestWhoamiMissingUser, self).__init__( - config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) @@ -347,15 +321,11 @@ class TestWhoamiMissingCreds(TestLoginBase): CONFIG_FILE = '/tmp/logintest.cfg' - CONFIG_CONTENT = (""" + CONFIG_CONTENTS = (""" [nonsense] foo = bar """) - def __init__(self, *args, **kwargs): - super(TestWhoamiMissingCreds, self).__init__( - config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) @@ -376,16 +346,12 @@ class TestWhoamiUncaughtException(TestLoginBase): USERNAME = 'st2foouser' - CONFIG_CONTENT = (""" + CONFIG_CONTENTS = (""" [credentials] username = %s password = Password1! """ % USERNAME) - def __init__(self, *args, **kwargs): - super(TestWhoamiUncaughtException, self).__init__( - config_contents=self.CONFIG_CONTENT, config_file=self.CONFIG_FILE) - @mock.patch.object( requests, 'post', mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) From 339af8950e799ae97c946675126c84391c1a628b Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 10:04:20 +0100 Subject: [PATCH 10/13] For safety reason use tempfile. --- st2client/tests/unit/test_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index a4069474e0..db426258f3 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -69,7 +69,7 @@ class TestLoginBase(base.BaseCLITestCase): """ DOTST2_PATH = os.path.expanduser('~/.st2/') - CONFIG_FILE = '/tmp/logintest.cfg' + CONFIG_FILE = tempfile.mkstemp(suffix='st2.conf') CONFIG_CONTENTS = None def __init__(self, *args, **kwargs): From 2602577b2935a587e79744db01eec0f347365e0b Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 10:05:28 +0100 Subject: [PATCH 11/13] Simplify code even further - we can just set a default value on the class variable. --- st2client/tests/unit/test_auth.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index db426258f3..334c0c8993 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -70,7 +70,11 @@ class TestLoginBase(base.BaseCLITestCase): DOTST2_PATH = os.path.expanduser('~/.st2/') CONFIG_FILE = tempfile.mkstemp(suffix='st2.conf') - CONFIG_CONTENTS = None + CONFIG_CONTENTS = """ + [credentials] + username = olduser + password = Password1! + """ def __init__(self, *args, **kwargs): super(TestLoginBase, self).__init__(*args, **kwargs) @@ -88,17 +92,8 @@ def setUp(self): os.remove(self.CONFIG_FILE) with open(self.CONFIG_FILE, 'w') as cfg: - # If a test passes in it's own config, we write that instead - if self.CONFIG_CONTENTS: - for line in self.CONFIG_CONTENTS.split('\n'): - cfg.write("%s\n" % line.strip()) - else: - - # Default config for most tests - cfg.write('[credentials]\n') - # Using 'olduser' so we can assert this has changed at the end - cfg.write('username = olduser\n') - cfg.write('password = Password1!\n') + for line in self.CONFIG_CONTENTS.split('\n'): + cfg.write('%s\n' % line.strip()) def tearDown(self): super(TestLoginBase, self).tearDown() From 380b958dd52bb10934e8cc43e9fb19d585262b34 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 13:36:49 +0100 Subject: [PATCH 12/13] Make sure we reset self.stdout and self.stderr on each setUp call otherwise it will be re-used when multiple class inherit from this base class which is undesired since we always want to start with a clean state. --- st2client/tests/base.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/st2client/tests/base.py b/st2client/tests/base.py index 63927f5abe..49d2fa6fa8 100644 --- a/st2client/tests/base.py +++ b/st2client/tests/base.py @@ -89,6 +89,10 @@ def setUp(self): os.environ['ST2_CLI_SKIP_CONFIG'] = '1' if self.capture_output: + # Make sure we reset it for each test class instance + self.stdout = six.moves.StringIO() + self.stderr = six.moves.StringIO() + sys.stdout = self.stdout sys.stderr = self.stderr From 072bd3ddcaa4e4e4368346902b7d74b75542a940 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 13 Feb 2017 13:38:48 +0100 Subject: [PATCH 13/13] Replace CaptureStdout with existing stdout and stderr capture mechanism. --- st2client/tests/unit/test_auth.py | 52 +++++++++---------------------- 1 file changed, 15 insertions(+), 37 deletions(-) diff --git a/st2client/tests/unit/test_auth.py b/st2client/tests/unit/test_auth.py index 334c0c8993..1e4c72c664 100644 --- a/st2client/tests/unit/test_auth.py +++ b/st2client/tests/unit/test_auth.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import sys import os import uuid import json @@ -23,8 +22,6 @@ import argparse import logging -from six import StringIO - from tests import base from st2client import shell from st2client.models.core import add_auth_token_to_kwargs_from_env @@ -42,23 +39,6 @@ } -class CaptureStdout(list): - """This is a bit of borrowed code to make it easier to read from stdout - - It's meant to be used within a context manager. - """ - - def __enter__(self): - self._stdout = sys.stdout - sys.stdout = self._stringio = StringIO() - return self - - def __exit__(self, *args): - self.extend(self._stringio.getvalue().splitlines()) - del self._stringio # free up some memory - sys.stdout = self._stdout - - class TestLoginBase(base.BaseCLITestCase): """ A base class for testing related to 'st2 login' commands @@ -68,6 +48,8 @@ class TestLoginBase(base.BaseCLITestCase): on duplicate code in each test class """ + capture_output = True + DOTST2_PATH = os.path.expanduser('~/.st2/') CONFIG_FILE = tempfile.mkstemp(suffix='st2.conf') CONFIG_CONTENTS = """ @@ -133,7 +115,6 @@ def runTest(self): with open(self.CONFIG_FILE, 'r') as config_file: for line in config_file.readlines(): - # Make sure certain values are not present self.assertFalse('password' in line) self.assertFalse('olduser' in line) @@ -256,10 +237,9 @@ def runTest(self, mock_gp): mock_gp.getpass = mock.MagicMock(side_effect=Exception) self.shell.run(args) - with CaptureStdout() as output: - retcode = self.shell.run(args) + retcode = self.shell.run(args) - self.assertTrue("Failed to log in as %s" % expected_username in output[0]) + self.assertTrue('Failed to log in as %s' % expected_username in self.stdout.getvalue()) self.assertEqual(retcode, 0) @@ -282,11 +262,10 @@ def runTest(self): '''Test 'st2 whoami' functionality ''' - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) self.assertEqual(retcode, 0) - self.assertTrue(self.USERNAME in output[0]) + self.assertTrue(self.USERNAME in self.stdout.getvalue()) class TestWhoamiMissingUser(TestLoginBase): @@ -305,10 +284,10 @@ def runTest(self): '''Test 'st2 whoami' functionality with a missing username ''' - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + self.assertEqual('Unable to retrieve currently logged-in user', + self.stdout.getvalue().strip()) self.assertEqual(retcode, 0) @@ -328,10 +307,9 @@ def runTest(self): '''Test 'st2 whoami' functionality with a missing credentials section ''' - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - - self.assertEqual('Unable to retrieve currently logged-in user', output[0]) + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + self.assertEqual('Unable to retrieve currently logged-in user', + self.stdout.getvalue().strip()) self.assertEqual(retcode, 0) @@ -358,10 +336,10 @@ def runTest(self, mock_cli): # Only mocking "BaseCLIApp" here in order to generate an exception for this specific test mock_cli.return_value._get_config_file_path = mock.MagicMock(side_effect=Exception) - with CaptureStdout() as output: - retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) + retcode = self.shell.run(['--config', self.CONFIG_FILE, 'whoami']) - self.assertEqual("Unable to retrieve currently logged-in user", output[0]) + self.assertEqual('Unable to retrieve currently logged-in user', + self.stdout.getvalue().strip()) self.assertEqual(retcode, 0)