From 6c95678a7624be4fc68b9a4664b26eb43b146353 Mon Sep 17 00:00:00 2001 From: Manik Bindlish Date: Thu, 25 Oct 2018 06:59:55 +0000 Subject: [PATCH] Handling invalid path of workspace register and move This PS will fix the invalid value handling. Error will be raised if no/blank value will be specified for workspace register and workspace move for path parameter. Change-Id: I0d9956cac27fd4dbb527fd865aa152e4724c01f9 Closes-Bug: #1799883 Partially-Implements: blueprint tempest-cli-unit-test-coverage --- .../notes/bug-1799883-6ea95fc64f70c9ef.yaml | 7 ++++ tempest/cmd/workspace.py | 9 +++-- tempest/tests/cmd/test_workspace.py | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1799883-6ea95fc64f70c9ef.yaml diff --git a/releasenotes/notes/bug-1799883-6ea95fc64f70c9ef.yaml b/releasenotes/notes/bug-1799883-6ea95fc64f70c9ef.yaml new file mode 100644 index 0000000000..630908fb0e --- /dev/null +++ b/releasenotes/notes/bug-1799883-6ea95fc64f70c9ef.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed bug #1799883. ``tempest workspace register`` and ``tempest workspace move`` CLI + will now validate the value of the ``--path`` CLI argument and raise an exception if + it is None or empty string. Earlier both CLI actions were accepting None or empty path + which was confusing. diff --git a/tempest/cmd/workspace.py b/tempest/cmd/workspace.py index d276bde43d..d0c4b281a0 100644 --- a/tempest/cmd/workspace.py +++ b/tempest/cmd/workspace.py @@ -94,7 +94,7 @@ class WorkspaceManager(object): @lockutils.synchronized('workspaces', external=True) def move_workspace(self, name, path): self._populate() - path = os.path.abspath(os.path.expanduser(path)) + path = os.path.abspath(os.path.expanduser(path)) if path else path self._name_exists(name) self._validate_path(path) self.workspaces[name] = path @@ -115,6 +115,7 @@ class WorkspaceManager(object): @lockutils.synchronized('workspaces', external=True) def remove_workspace_directory(self, workspace_path): + self._validate_path(workspace_path) shutil.rmtree(workspace_path) @lockutils.synchronized('workspaces', external=True) @@ -136,6 +137,10 @@ class WorkspaceManager(object): sys.exit(1) def _validate_path(self, path): + if not path: + print("None or empty path is specified for workspace." + " Please specify correct workspace path.") + sys.exit(1) if not os.path.exists(path): print("Path does not exist.") sys.exit(1) @@ -144,7 +149,7 @@ class WorkspaceManager(object): def register_new_workspace(self, name, path, init=False): """Adds the new workspace and writes out the new workspace config""" self._populate() - path = os.path.abspath(os.path.expanduser(path)) + path = os.path.abspath(os.path.expanduser(path)) if path else path # This only happens when register is called from outside of init if not init: self._validate_path(path) diff --git a/tempest/tests/cmd/test_workspace.py b/tempest/tests/cmd/test_workspace.py index a256368265..65481de0a5 100644 --- a/tempest/tests/cmd/test_workspace.py +++ b/tempest/tests/cmd/test_workspace.py @@ -140,6 +140,17 @@ class TestTempestWorkspaceManager(TestTempestWorkspaceBase): self.assertEqual( self.workspace_manager.get_workspace(self.name), new_path) + def test_workspace_manager_move_no_workspace_path(self): + new_path = "" + with patch('sys.stdout', new_callable=StringIO) as mock_stdout: + ex = self.assertRaises(SystemExit, + self.workspace_manager.move_workspace, + self.name, new_path) + self.assertEqual(1, ex.code) + self.assertEqual(mock_stdout.getvalue(), + "None or empty path is specified for workspace." + " Please specify correct workspace path.\n") + def test_workspace_manager_remove_entry(self): self.workspace_manager.remove_workspace_entry(self.name) self.assertIsNone(self.workspace_manager.get_workspace(self.name)) @@ -149,6 +160,18 @@ class TestTempestWorkspaceManager(TestTempestWorkspaceBase): self.workspace_manager.remove_workspace_directory(path) self.assertIsNone(self.workspace_manager.get_workspace(self.name)) + def test_workspace_manager_remove_directory_no_path(self): + no_path = "" + with patch('sys.stdout', new_callable=StringIO) as mock_stdout: + ex = self.assertRaises(SystemExit, + self.workspace_manager. + remove_workspace_directory, + no_path) + self.assertEqual(1, ex.code) + self.assertEqual(mock_stdout.getvalue(), + "None or empty path is specified for workspace." + " Please specify correct workspace path.\n") + def test_path_expansion(self): name = data_utils.rand_uuid() path = os.path.join("~", name) @@ -207,3 +230,15 @@ class TestTempestWorkspaceManager(TestTempestWorkspaceBase): self.assertEqual(mock_stdout.getvalue(), "None or empty name is specified." " Please specify correct name for workspace.\n") + + def test_register_new_workspace_no_path(self): + no_path = "" + with patch('sys.stdout', new_callable=StringIO) as mock_stdout: + ex = self.assertRaises(SystemExit, + self.workspace_manager. + register_new_workspace, + self.name, no_path) + self.assertEqual(1, ex.code) + self.assertEqual(mock_stdout.getvalue(), + "None or empty path is specified for workspace." + " Please specify correct workspace path.\n")