Merge "Fix checks for required parameters for nodegroup commands"

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(

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
         )
 

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)