From 21491dff81aa5a2448406b55ec58fff19dcf4c5f Mon Sep 17 00:00:00 2001 From: Manik Bindlish Date: Fri, 14 Dec 2018 06:58:42 +0000 Subject: [PATCH] Checking config file actually exist or not in tempest run This PS will check config file passed from command line is actually existing or not in tempest run command. If file exists, only then it will go for set config path otherwise will fail with value error. Also adding and modifying unit test cases for this change. Partially-Implements: blueprint tempest-cli-unit-test-coverage Change-Id: I09d756be69cb3a9be8d0638c41d45a089d62b238 Closes-Bug: #1808473 --- .../notes/bug-1808473-54ada26ab78e7b02.yaml | 7 ++++ tempest/cmd/run.py | 10 ++++- tempest/tests/cmd/test_run.py | 38 ++++++++++++++----- 3 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/bug-1808473-54ada26ab78e7b02.yaml diff --git a/releasenotes/notes/bug-1808473-54ada26ab78e7b02.yaml b/releasenotes/notes/bug-1808473-54ada26ab78e7b02.yaml new file mode 100644 index 0000000000..c28019832d --- /dev/null +++ b/releasenotes/notes/bug-1808473-54ada26ab78e7b02.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed bug #1808473. ``tempest run`` CLI will error if a non-exist config file is + input to parameter --config-file. Earlier non-exist config value was silently + getting ignored and the default config file was used instead which used to give + false behavior to the user on using the passed config file. diff --git a/tempest/cmd/run.py b/tempest/cmd/run.py index bb3fe63c8b..77d44963cb 100644 --- a/tempest/cmd/run.py +++ b/tempest/cmd/run.py @@ -103,6 +103,9 @@ from tempest.cmd import workspace from tempest.common import credentials_factory as credentials from tempest import config +if six.PY2: + # Python 2 has not FileNotFoundError exception + FileNotFoundError = IOError CONF = config.CONF SAVED_STATE_JSON = "saved_state.json" @@ -112,7 +115,12 @@ class TempestRun(command.Command): def _set_env(self, config_file=None): if config_file: - CONF.set_config_path(os.path.abspath(config_file)) + if os.path.exists(os.path.abspath(config_file)): + CONF.set_config_path(os.path.abspath(config_file)) + else: + raise FileNotFoundError( + "Config file: %s doesn't exist" % config_file) + # NOTE(mtreinish): This is needed so that stestr doesn't gobble up any # stacktraces on failure. if 'TESTR_PDB' in os.environ: diff --git a/tempest/tests/cmd/test_run.py b/tempest/tests/cmd/test_run.py index 00f8bc57e8..0e00d94ce1 100644 --- a/tempest/tests/cmd/test_run.py +++ b/tempest/tests/cmd/test_run.py @@ -29,6 +29,10 @@ from tempest import config from tempest.lib.common.utils import data_utils from tempest.tests import base +if six.PY2: + # Python 2 has not FileNotFoundError exception + FileNotFoundError = IOError + DEVNULL = open(os.devnull, 'wb') atexit.register(DEVNULL.close) @@ -244,14 +248,22 @@ class TestConfigPathCheck(base.TestCase): # getting set in os environment when some data has passed to # set the environment. - self.run_cmd._set_env("/fakedir/fakefile") - self.assertEqual("/fakedir/fakefile", CONF._path) - self.assertIn('TEMPEST_CONFIG_DIR', os.environ) - self.assertEqual("/fakedir/fakefile", - os.path.join(os.environ['TEMPEST_CONFIG_DIR'], - os.environ['TEMPEST_CONFIG'])) + _, path = tempfile.mkstemp() + self.addCleanup(os.remove, path) - def test_tempest_run_set_config_no_path(self): + self.run_cmd._set_env(path) + self.assertEqual(path, CONF._path) + self.assertIn('TEMPEST_CONFIG_DIR', os.environ) + self.assertEqual(path, os.path.join(os.environ['TEMPEST_CONFIG_DIR'], + os.environ['TEMPEST_CONFIG'])) + + def test_tempest_run_set_config_no_exist_path(self): + path = "fake/path" + self.assertRaisesRegex(FileNotFoundError, + 'Config file: .* doesn\'t exist', + self.run_cmd._set_env, path) + + def test_tempest_run_no_config_path(self): # Note: (mbindlish) This test is created for the bug id: 1783751 # Checking TEMPEST_CONFIG_DIR and TEMPEST_CONFIG should have no value # in os environment when no data has passed to set the environment. @@ -313,13 +325,15 @@ class TestTakeAction(base.TestCase): def test_config_file_specified(self): self._setup_test_dirs() + _, path = tempfile.mkstemp() + self.addCleanup(os.remove, path) tempest_run = run.TempestRun(app=mock.Mock(), app_args=mock.Mock()) parsed_args = mock.Mock() parsed_args.workspace = None parsed_args.state = None parsed_args.list_tests = False - parsed_args.config_file = '.stestr.conf' + parsed_args.config_file = path with mock.patch('stestr.commands.run_command') as m: m.return_value = 0 @@ -341,13 +355,15 @@ class TestTakeAction(base.TestCase): def test_config_file_workspace_registered(self): self._setup_test_dirs() + _, path = tempfile.mkstemp() + self.addCleanup(os.remove, path) tempest_run = run.TempestRun(app=mock.Mock(), app_args=mock.Mock()) parsed_args = mock.Mock() parsed_args.workspace = self.name parsed_args.workspace_path = self.store_file parsed_args.state = None parsed_args.list_tests = False - parsed_args.config_file = '.stestr.conf' + parsed_args.config_file = path with mock.patch('stestr.commands.run_command') as m: m.return_value = 0 @@ -406,13 +422,15 @@ class TestTakeAction(base.TestCase): @mock.patch('tempest.cmd.run.TempestRun._init_state') def test_no_workspace_config_file_state_true(self, mock_init_state): self._setup_test_dirs() + _, path = tempfile.mkstemp() + self.addCleanup(os.remove, path) tempest_run = run.TempestRun(app=mock.Mock(), app_args=mock.Mock()) parsed_args = mock.Mock() parsed_args.workspace = None parsed_args.workspace_path = self.store_file parsed_args.state = True parsed_args.list_tests = False - parsed_args.config_file = '.stestr.conf' + parsed_args.config_file = path with mock.patch('stestr.commands.run_command') as m: m.return_value = 0