From 2bd66f1a4878b0a6ba05da3705df991f00999273 Mon Sep 17 00:00:00 2001 From: Artem Roma Date: Tue, 29 Sep 2015 14:03:58 +0300 Subject: [PATCH] Fix checks for required parameters for nodegroup commands Change-Id: Ica428a4508006d51a7bec877c0239234c6b459c2 Related-Bug: #1402263 --- fuelclient/cli/actions/nodegroup.py | 7 ++- fuelclient/tests/functional/v1/test_client.py | 2 +- fuelclient/tests/unit/v1/test_nodegroups.py | 60 ++++++++++--------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/fuelclient/cli/actions/nodegroup.py b/fuelclient/cli/actions/nodegroup.py index 5376374..ff43366 100644 --- a/fuelclient/cli/actions/nodegroup.py +++ b/fuelclient/cli/actions/nodegroup.py @@ -58,6 +58,7 @@ class NodeGroupAction(Action): (None, self.list) ) + @check_all("env", "name") def create(self, params): """Create a new node group fuel --env 1 nodegroup --create --name "group 1" @@ -103,11 +104,11 @@ class NodeGroupAction(Action): .format(id=n.id) ) - @check_all("env", "node", "group") + @check_all("node", "group") def assign(self, params): """Assign nodes to specified node group: - fuel --env 1 nodegroup --assign --node 1 --group 1 - fuel --env 1 nodegroup --assign --node 2,3,4 --group 1 + fuel nodegroup --assign --node 1 --group 1 + fuel nodegroup --assign --node 2,3,4 --group 1 """ if len(params.group) > 1: raise ActionException( diff --git a/fuelclient/tests/functional/v1/test_client.py b/fuelclient/tests/functional/v1/test_client.py index c7a799b..da443de 100644 --- a/fuelclient/tests/functional/v1/test_client.py +++ b/fuelclient/tests/functional/v1/test_client.py @@ -257,7 +257,7 @@ class TestHandlers(base.BaseTestCase): '"id": 1', '"group_id": 2'] self.check_all_in_msg( - "nodegroup --env 1 --assign --group 2 --node 1 --debug", + "nodegroup --assign --group 2 --node 1 --debug", msg ) diff --git a/fuelclient/tests/unit/v1/test_nodegroups.py b/fuelclient/tests/unit/v1/test_nodegroups.py index 535ba0a..6644430 100644 --- a/fuelclient/tests/unit/v1/test_nodegroups.py +++ b/fuelclient/tests/unit/v1/test_nodegroups.py @@ -71,6 +71,26 @@ class TestNodeGroupActions(base.UnitTestCase): self.assertTrue(mget.called) + def _check_required_message_for_commands(self, err_msg, commands): + for cmd in commands: + with mock.patch("sys.stderr") as m_stderr: + with self.assertRaises(SystemExit): + self.execute(cmd) + + m_stderr.write.assert_called_with(err_msg) + + def test_create_nodegroup_arguments_required(self, mreq): + err_msg = '"--env" and "--name" required!\n' + + env_not_present = ['fuel', 'nodegroup', '--create', + '--name', 'test'] + + name_not_present = ['fuel', '--env', str(self.env['id']), + 'nodegroup', '--create'] + + self._check_required_message_for_commands( + err_msg, (env_not_present, name_not_present)) + def test_delete_nodegroup(self, mreq): path = self.req_base_path + str(self.env['id']) + '/' mget = mreq.get(path, json={'name': 'test group'}) @@ -91,20 +111,17 @@ class TestNodeGroupActions(base.UnitTestCase): def test_delete_nodegroup_group_arg_required(self, mreq): err_msg = '"--group" required!\n' - with mock.patch('sys.stderr') as m_stderr: - with self.assertRaises(SystemExit): - self.execute(['fuel', 'nodegroup', '--delete', - '--default']) - - msg = m_stderr.write.call_args[0][0] - self.assertEqual(msg, err_msg) + self._check_required_message_for_commands( + err_msg, + (['fuel', 'nodegroup', '--delete', '--default'],) + ) def test_assign_nodegroup_fails_w_multiple_groups(self, mreq): err_msg = "Nodes can only be assigned to one node group.\n" with mock.patch("sys.stderr") as m_stderr: with self.assertRaises(SystemExit): self.execute(['fuel', 'nodegroup', '--assign', '--node', '1', - '--env', str(self.env['id']), '--group', '2,3']) + '--group', '2,3']) msg = m_stderr.write.call_args[0][0] self.assertEqual(msg, err_msg) @@ -112,32 +129,21 @@ class TestNodeGroupActions(base.UnitTestCase): @mock.patch('fuelclient.objects.nodegroup.NodeGroup.assign') def test_assign_nodegroup(self, m_req, m_assign): self.execute(['fuel', 'nodegroup', '--assign', '--node', '1', - '--env', str(self.env['id']), '--group', '2']) + '--group', '2']) m_assign.assert_called_with([1]) self.execute(['fuel', 'nodegroup', '--assign', '--node', '1,2,3', - '--env', str(self.env['id']), '--group', '2']) + '--group', '2']) m_assign.assert_called_with([1, 2, 3]) def test_node_group_assign_arguments_required(self, mreq): - err_msg = '"--env", "--node" and "--group" required!\n' + err_msg = '"--node" and "--group" required!\n' - node_not_present_cmd = ['fuel', 'nodegroup', '--env', - str(self.env['id']), '--assign', '--group', - '1'] - group_not_present_cmd = ['fuel', 'nodegroup', '--env', - str(self.env['id']), '--assign', + node_not_present_cmd = ['fuel', 'nodegroup', '--assign', + '--group', '1'] + group_not_present_cmd = ['fuel', 'nodegroup', '--assign', '--node', '1'] - env_not_present_cmd = ['fuel', 'nodegroup', '--assign', '--node', '1', - '--group', '1'] - commands = (node_not_present_cmd, group_not_present_cmd, - env_not_present_cmd) + commands = (node_not_present_cmd, group_not_present_cmd) - for cmd in commands: - with mock.patch("sys.stderr") as m_stderr: - with self.assertRaises(SystemExit): - self.execute(cmd) - - msg = m_stderr.write.call_args[0][0] - self.assertEqual(msg, err_msg) + self._check_required_message_for_commands(err_msg, commands)