Don't hide exceptions when they happen

This makes things hard to debug.
Fixed tests too.

Change-Id: I435c269702995a243728d207f26487ba56aeacba
Partial-Bug: #1484430

fuelclient/cli/error.py

@@ -115,9 +115,6 @@ def exceptions_decorator(func):
             /etc/fuel/client/config.yaml""")
         except FuelClientException as exc:
             exit_with_error(exc.message)
-        # not all responses return data
-        except ValueError:
-            return {}
 
     return wrapper
 

fuelclient/client.py

@@ -109,6 +109,9 @@ class Client(object):
         resp = requests.delete(url, headers=headers)
         resp.raise_for_status()
 
+        if resp.status_code == 204:
+            return {}
+
         return resp.json()
 
     def put_request(self, api, data):

fuelclient/tests/common/unit/test_network_template.py

@@ -116,22 +116,22 @@ class TestNetworkTemplate(base.UnitTestCase):
         self.mocker.stop()
 
     def test_upload_action(self):
-        self.mocker.put(self.req_path)
+        mput = self.mocker.put(self.req_path, json={})
         test_command = [
             'fuel', 'network-template', '--env', str(self.env_id), '--upload']
 
         m_open = mock.mock_open(read_data=YAML_TEMPLATE)
-        with mock.patch('__builtin__.open', m_open, create=True):
+        with mock.patch('fuelclient.cli.serializers.open',
+                        m_open,
+                        create=True):
             self.execute(test_command)
 
-        self.assertEqual(self.mocker.last_request.method, 'PUT')
-        self.assertEqual(self.mocker.last_request.path, self.req_path)
-        self.assertEqual(self.mocker.last_request.json(),
-                         json.loads(JSON_TEMPLATE))
+        self.assertTrue(mput.called)
+        self.assertEqual(mput.last_request.json(), json.loads(JSON_TEMPLATE))
         m_open().read.assert_called_once_with()
 
     def test_download_action(self):
-        self.mocker.get(self.req_path, text=JSON_TEMPLATE)
+        mget = self.mocker.get(self.req_path, text=JSON_TEMPLATE)
 
         test_command = [
             'fuel', 'network-template', '--env', str(self.env_id),
@@ -142,15 +142,14 @@ class TestNetworkTemplate(base.UnitTestCase):
                         create=True):
             self.execute(test_command)
 
-        self.assertEqual(self.mocker.last_request.method, 'GET')
-        self.assertEqual(self.mocker.last_request.path, self.req_path)
+        self.assertTrue(mget.called)
 
         written_yaml = yaml.safe_load(m_open().write.mock_calls[0][1][0])
         expected_yaml = yaml.safe_load(YAML_TEMPLATE)
         self.assertEqual(written_yaml, expected_yaml)
 
     def test_delete_action(self):
-        self.mocker.delete(self.req_path, json={})
+        mdelete = self.mocker.delete(self.req_path, json={})
 
         cmd = ['fuel', 'network-template', '--env', str(self.env_id),
                '--delete']
@@ -158,8 +157,7 @@ class TestNetworkTemplate(base.UnitTestCase):
         with mock.patch('sys.stdout', new=six.StringIO()) as m_out:
             self.execute(cmd)
 
-        self.assertEqual(self.mocker.last_request.method, 'DELETE')
-        self.assertEqual(self.mocker.last_request.path, self.req_path)
+        self.assertTrue(mdelete.called)
 
         msg = ("Network template configuration for environment id={0}"
                " has been deleted.".format(self.env_id))

fuelclient/tests/utils/fake_env.py

@@ -16,7 +16,8 @@
 
 
 def get_fake_env(name=None, status=None, release_id=None,
-                 fuel_version=None, pending_release=None, env_id=None):
+                 fuel_version=None, pending_release=None, env_id=None,
+                 net_provider=None):
     """Create a random fake environment
 
     Returns the serialized and parametrized representation of a dumped Fuel
@@ -27,7 +28,7 @@ def get_fake_env(name=None, status=None, release_id=None,
             'is_customized': False,
             'release_id': release_id or 1,
             'name': name or 'fake_env',
-            'net_provider': 'neutron',
+            'net_provider': net_provider or 'neutron',
             'net_segment_type': 'gre',
             'fuel_version': fuel_version or '5.1',
             'pending_release_id': pending_release,

fuelclient/tests/v1/unit/test_network_groups.py

@@ -16,6 +16,7 @@ import mock
 import requests_mock
 
 from fuelclient.tests import base
+from fuelclient.tests.utils import fake_network_group
 
 
 @requests_mock.mock()
@@ -27,46 +28,50 @@ class TestNetworkGroupActions(base.UnitTestCase):
         self.env_id = 42
         self.req_base_path = '/api/v1/networks/'
 
+        self.ng = fake_network_group.get_fake_network_group()
+
     def test_list_network_groups(self, mreq):
-        mreq.get(self.req_base_path)
+        mget = mreq.get(self.req_base_path, json={})
         list_commands = [
             ['fuel', 'network-group', '--list'], ['fuel', 'network-group']]
 
         for cmd in list_commands:
             self.execute(cmd)
-            self.assertEqual(mreq.last_request.method, 'GET')
-            self.assertEqual(mreq.last_request.path, self.req_base_path)
+            self.assertTrue(mget.called)
 
     def test_list_network_groups_filtering(self, mreq):
-        mreq.get(self.req_base_path)
+        mget = mreq.get(self.req_base_path, json={})
 
-        self.execute(['fuel', 'network-group', '--node-group', '1'])
+        self.execute(
+            ['fuel', 'network-group', '--node-group', str(self.ng['id'])]
+        )
 
-        self.assertEqual(mreq.last_request.method, 'GET')
-        self.assertEqual(mreq.last_request.path, self.req_base_path)
+        self.assertTrue(mget.called)
 
     def create_network_group(self, mreq, cmd):
-        mreq.post(self.req_base_path)
+        mpost = mreq.post(self.req_base_path, json={
+            'id': self.ng['id'],
+            'name': self.ng['name'],
+        })
         self.execute(cmd)
 
-        call_data = mreq.last_request.json()
-        self.assertEqual(1, call_data['group_id'])
-        self.assertEqual("test network", call_data['name'])
+        call_data = mpost.last_request.json()
+        self.assertEqual(self.ng['id'], call_data['group_id'])
+        self.assertEqual(self.ng['name'], call_data['name'])
 
-        self.assertEqual(mreq.last_request.method, 'POST')
-        self.assertEqual(mreq.last_request.path, self.req_base_path)
+        self.assertTrue(mpost.called)
 
         return call_data
 
     def test_create_network_group(self, mreq):
-        cmd = ['fuel', 'network-group', '--create', '--cidr', '10.0.0.0/24',
-               '--name', 'test network', '--node-group', '1']
+        cmd = ['fuel', 'network-group', '--create', '--cidr', self.ng['cidr'],
+               '--name', self.ng['name'], '--node-group', str(self.ng['id'])]
         self.create_network_group(mreq, cmd)
 
     def test_create_network_group_w_meta(self, mreq):
-        cmd = ['fuel', 'network-group', '--create', '--cidr', '10.0.0.0/24',
-               '--name', 'test network', '--node-group', '1', '--meta',
-               '{"ip_ranges": ["10.0.0.2", "10.0.0.254"]}']
+        cmd = ['fuel', 'network-group', '--create', '--cidr', self.ng['cidr'],
+               '--name', self.ng['name'], '--node-group', str(self.ng['id']),
+               '--meta', '{"ip_ranges": ["10.0.0.2", "10.0.0.254"]}']
         self.create_network_group(mreq, cmd)
 
         meta = mreq.last_request.json()['meta']
@@ -91,47 +96,43 @@ class TestNetworkGroupActions(base.UnitTestCase):
 
     def test_delete_network_group(self, mreq):
         path = self.req_base_path + str(self.env_id) + '/'
-        mreq.delete(path)
+        mdelete = mreq.delete(path, status_code=204)
         self.execute(
             ['fuel', 'network-group', '--delete',
              '--network', str(self.env_id)])
 
-        self.assertEqual(mreq.last_request.method, 'DELETE')
-        self.assertEqual(mreq.last_request.path, path)
+        self.assertTrue(mdelete.called)
 
     def test_network_group_duplicate_name(self, mreq):
-        mreq.post(self.req_base_path, status_code=409)
+        mpost = mreq.post(self.req_base_path, status_code=409)
 
         with mock.patch("sys.stderr") as m_stderr:
             with self.assertRaises(SystemExit):
                 self.execute(
                     ['fuel', 'network-group', '--create', '--cidr',
-                     '10.0.0.0/24', '--name', 'test network', '--node-group',
-                     '1'])
+                     self.ng['cidr'], '--name', self.ng['name'],
+                     '--node-group', str(self.ng['id'])])
 
         self.assertIn("409 Client Error", m_stderr.write.call_args[0][0])
-        self.assertEqual(mreq.last_request.method, 'POST')
-        self.assertEqual(mreq.last_request.path, self.req_base_path)
+        self.assertTrue(mpost.called)
 
     def test_set_network_group(self, mreq):
         path = self.req_base_path + str(self.env_id) + '/'
-        mreq.put(path)
+        mput = mreq.put(path, json={})
         self.execute([
-            'fuel', 'network-group', '--set', '--network', '42',
-            '--name', 'new name'])
+            'fuel', 'network-group', '--set', '--network', str(self.ng['id']),
+            '--name', self.ng['name']])
 
-        self.assertEqual(mreq.last_request.method, 'PUT')
-        self.assertEqual(mreq.last_request.path, path)
+        self.assertTrue(mput.called)
 
     def test_set_network_group_meta(self, mreq):
         path = self.req_base_path + str(self.env_id) + '/'
-        mreq.put(path)
+        mput = mreq.put(path, json={})
         self.execute([
-            'fuel', 'network-group', '--set', '--network', '42',
+            'fuel', 'network-group', '--set', '--network', str(self.ng['id']),
             '--meta', '{"ip_ranges": ["10.0.0.2", "10.0.0.254"]}'])
 
-        self.assertEqual(mreq.last_request.method, 'PUT')
-        self.assertEqual(mreq.last_request.path, path)
+        self.assertTrue(mput.called)
 
         meta = mreq.last_request.json()['meta']
         self.assertEqual(meta['ip_ranges'], ["10.0.0.2", "10.0.0.254"])

fuelclient/tests/v1/unit/test_nodegroups.py

@@ -16,6 +16,8 @@ import mock
 import requests_mock
 
 from fuelclient.tests import base
+from fuelclient.tests.utils import fake_env
+from fuelclient.tests.utils import fake_network_group
 
 
 @requests_mock.mock()
@@ -24,46 +26,58 @@ class TestNodeGroupActions(base.UnitTestCase):
     def setUp(self):
         super(TestNodeGroupActions, self).setUp()
 
-        self.env_id = 42
+        self.env = fake_env.get_fake_env(net_provider='neutron')
         self.req_base_path = '/api/v1/nodegroups/'
+        self.ng = fake_network_group.get_fake_network_group()
 
     def test_list_nodegroups(self, mreq):
-        mreq.get(self.req_base_path)
+        mget = mreq.get(self.req_base_path, json=[])
         self.execute(['fuel', 'nodegroup', '--list'])
-        self.assertEqual(mreq.last_request.method, 'GET')
-        self.assertEqual(mreq.last_request.path, self.req_base_path)
+
+        self.assertTrue(mget.called)
 
     def test_create_nodegroup(self, mreq):
-        mreq.post(self.req_base_path)
+        neutron_url = \
+            '/api/v1/clusters/{0}/network_configuration/neutron'.format(
+                self.env['id']
+            )
+
+        mreq.get('/api/v1/clusters/{0}/'.format(self.env['id']),
+                 json={
+                     'id': self.env['id'],
+                     'net_provider': self.env['net_provider'],
+                 })
+        mpost = mreq.post(self.req_base_path, json={
+            'id': self.ng['id'],
+            'name': self.ng['name'],
+        })
+        mget = mreq.get(neutron_url, json={'networking_parameters': {}})
         self.execute(['fuel', 'nodegroup', '--create',
-                      '--name', 'test group', '--env', str(self.env_id)])
+                      '--name', self.ng['name'], '--env', str(self.env['id'])])
 
-        call_data = mreq.last_request.json()
-        self.assertEqual(self.env_id, call_data['cluster_id'])
-        self.assertEqual('test group', call_data['name'])
+        call_data = mpost.last_request.json()
+        self.assertEqual(self.env['id'], call_data['cluster_id'])
+        self.assertEqual(self.ng['name'], call_data['name'])
 
-        self.assertEqual(mreq.last_request.method, 'POST')
-        self.assertEqual(mreq.last_request.path, self.req_base_path)
+        self.assertTrue(mget.called)
 
     def test_delete_nodegroup(self, mreq):
-        path = self.req_base_path + str(self.env_id) + '/'
-        mreq.get(path, json={'name': 'test group'})
-        delete_path = self.req_base_path + str(self.env_id) + '/'
-        mreq.delete(delete_path)
+        path = self.req_base_path + str(self.env['id']) + '/'
+        mget = mreq.get(path, json={'name': 'test group'})
+        delete_path = self.req_base_path + str(self.env['id']) + '/'
+        mdelete = mreq.delete(delete_path, status_code=204)
         self.execute(['fuel', 'nodegroup', '--delete', '--group',
-                      str(self.env_id)])
-        self.assertEqual(mreq.request_history[-2].method, 'GET')
-        self.assertEqual(mreq.request_history[-2].path, path)
+                      str(self.env['id'])])
 
-        self.assertEqual(mreq.last_request.method, 'DELETE')
-        self.assertEqual(mreq.last_request.path, delete_path)
+        self.assertTrue(mget.called)
+        self.assertTrue(mdelete.called)
 
     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', '1', '--group', '2,3'])
+                              '--env', str(self.env['id']), '--group', '2,3'])
 
         msg = m_stderr.write.call_args[0][0]
         self.assertEqual(msg, err_msg)
@@ -71,9 +85,9 @@ 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', '1', '--group', '2'])
+                      '--env', str(self.env['id']), '--group', '2'])
         m_assign.assert_called_with([1])
 
         self.execute(['fuel', 'nodegroup', '--assign', '--node', '1,2,3',
-                      '--env', '1', '--group', '2'])
+                      '--env', str(self.env['id']), '--group', '2'])
         m_assign.assert_called_with([1, 2, 3])

fuelclient/tests/v2/unit/cli/test_engine.py

@@ -66,11 +66,11 @@ class BaseCLITest(base.UnitTestCase):
             m_stdin.readline.side_effect = commands
             self.exec_command()
 
-    @mock.patch('cliff.commandmanager.CommandManager.find_command')
-    def test_command_non_interactive(self, m_find_command):
+    @mock.patch('cliff.app.App.run_subcommand')
+    def test_command_non_interactive(self, m_run_command):
         args = ['help']
         self.exec_command(*args)
-        m_find_command.assert_called_once_with(args)
+        m_run_command.assert_called_once_with(args)
 
     @mock.patch.object(sys, 'stderr')
     @mock.patch('cliff.app.App.run_subcommand')