Added more functional tests, reworked the helper methods as per discussion with @pjbreaux

Author: wojtek0806

f5/bigip/mixins.py

@@ -39,11 +39,6 @@ class UnsupportedTmosVersion(F5SDKError):
     pass
 
 
-class MissingRequiredCommandParameter(F5SDKError):
-    """Various values MUST be provided to execute a command."""
-    pass
-
-
 class LazyAttributesRequired(F5SDKError):
     """Raised when a object accesses a lazy attribute that is not listed"""
     pass
@@ -257,11 +252,7 @@ def _exec_cmd(self, command, **kwargs):
         kwargs['command'] = command
         self._check_exclusive_parameters(**kwargs)
         requests_params = self._handle_requests_params(kwargs)
-        rset = self._meta_data['required_command_parameters']
-        check = self._check_required_parameters(rset, **kwargs)
-        if check:
-            error_message = 'Missing required params: {}'.format(check[1])
-            raise MissingRequiredCommandParameter(error_message)
+        self._check_command_parameters(**kwargs)
         session = self._meta_data['bigip']._meta_data['icr_session']
         response = session.post(
             self._meta_data['uri'], json=kwargs, **requests_params)

f5/bigip/resource.py

@@ -87,19 +87,18 @@
 from requests.exceptions import HTTPError
 
 
-class ExclusiveAttributesPresent(F5SDKError):
-    """Raises this when exclusive attributes are present
+class MissingRequiredCommandParameter(F5SDKError):
+    """Various values MUST be provided to execute a command."""
+    pass
 
-    during resource creation
-    """
+
+class ExclusiveAttributesPresent(F5SDKError):
+    """Raises this when exclusive attributes are present."""
     pass
 
 
 class MissingUpdateParameter(F5SDKError):
-    """Raises this when update requires specific
-
-    parameters together
-    """
+    """Raises this when update requires specific parameters together."""
     pass
 
 
@@ -209,9 +208,34 @@ def _check_load_parameters(self, **kwargs):
         :raises: MissingRequiredReadParameter
         """
         rset = self._meta_data['required_load_parameters']
-        check = self._check_required_parameters(rset, **kwargs)
+        check = self._missing_required_parameters(rset, **kwargs)
+        if check:
+            error_message = 'Missing required params: %s' % check
+            raise MissingRequiredReadParameter(error_message)
+
+    def _check_create_parameters(self, **kwargs):
+        """Params given to create should satisfy required params.
+
+        :params: kwargs
+        :raises: MissingRequiredCreateParameter
+        """
+        rset = self._meta_data['required_creation_parameters']
+        check = self._missing_required_parameters(rset, **kwargs)
         if check:
-            raise MissingRequiredReadParameter(check[1])
+            error_message = 'Missing required params: %s' % check
+            raise MissingRequiredCreationParameter(error_message)
+
+    def _check_command_parameters(self, **kwargs):
+        """Params given to exec_cmd should satisfy required params.
+
+        :params: kwargs
+        :raises: MissingRequiredCommandParameter
+        """
+        rset = self._meta_data['required_command_parameters']
+        check = self._missing_required_parameters(rset, **kwargs)
+        if check:
+            error_message = 'Missing required params: %s' % check
+            raise MissingRequiredCommandParameter(error_message)
 
     def _local_update(self, rdict):
         """Call this with a response dictionary to update instance attrs.
@@ -322,19 +346,19 @@ def _check_exclusive_parameters(self, **kwargs):
                 raise ExclusiveAttributesPresent(error)
 
     @staticmethod
-    def _check_required_parameters(rqset, **kwargs):
-        """Helper function to do operation on sets
+    def _missing_required_parameters(rqset, **kwargs):
+        """Helper function to do operation on sets.
+
+        Checks for any missing required parameters.
+        Returns non-empty or empty list. With empty
+        list being False.
 
-        ::returns bool and variable
+        ::returns list
         """
         key_set = set(kwargs.keys())
         required_minus_received = rqset - key_set
         if required_minus_received != set():
-            error_message = 'Missing required params: %s' % \
-                            required_minus_received
-            return True, error_message
-        else:
-            return False
+            return list(required_minus_received)
 
     @property
     def raw(self):
@@ -704,13 +728,7 @@ def _create(self, **kwargs):
             raise URICreationCollision(error)
         self._check_exclusive_parameters(**kwargs)
         requests_params = self._handle_requests_params(kwargs)
-
-        # Using helper method to check for required keys.
-        # Defining convenience variables to make code readable
-        rset = self._meta_data['required_creation_parameters']
-        check = self._check_required_parameters(rset, **kwargs)
-        if check:
-            raise MissingRequiredCreationParameter(check[1])
+        self._check_create_parameters(**kwargs)
 
         # Make convenience variable with short names for this method.
         _create_uri = self._meta_data['container']._meta_data['uri']

f5/bigip/test/test_resource.py

@@ -18,13 +18,16 @@
 
 from f5.bigip.resource import Collection
 from f5.bigip.resource import DeviceProvidesIncompatibleKey
+from f5.bigip.resource import ExclusiveAttributesPresent
 from f5.bigip.resource import GenerationMismatch
 from f5.bigip.resource import InvalidForceType
 from f5.bigip.resource import InvalidResource
 from f5.bigip.resource import KindTypeMismatch
+from f5.bigip.resource import MissingRequiredCommandParameter
 from f5.bigip.resource import MissingRequiredCreationParameter
 from f5.bigip.resource import MissingRequiredReadParameter
 from f5.bigip.resource import OrganizingCollection
+from f5.bigip.resource import PathElement
 from f5.bigip.resource import RequestParamKwargCollision
 from f5.bigip.resource import Resource
 from f5.bigip.resource import ResourceBase
@@ -96,7 +99,7 @@ def test_missing_required_creation_parameter(self):
         with pytest.raises(MissingRequiredCreationParameter) as MRCPEIO:
             r.create(partition="Common", name='CreateTest')
         assert MRCPEIO.value.message ==\
-            "Missing required params: set(['NONEMPTY'])"
+            "Missing required params: ['NONEMPTY']"
 
     def test_KindTypeMismatch(self):
         r = Resource(mock.MagicMock())
@@ -290,7 +293,7 @@ def test_missing_required_params(self):
         with pytest.raises(MissingRequiredReadParameter) as MRREIO:
             r.load(partition='Common', name='test_load')
         assert MRREIO.value.message ==\
-            "Missing required params: set(['IMPOSSIBLE'])"
+            "Missing required params: ['IMPOSSIBLE']"
 
     def test_requests_params_collision(self):
         r = Resource(mock.MagicMock())
@@ -443,3 +446,64 @@ def test_collection_s():
     }
     tc_s = Under_s(MC)
     assert tc_s._meta_data['uri'] == 'BASEURI/under/'
+
+
+class TestPathElement(object):
+
+    def test_missing_req_param_true(self):
+        p = PathElement(mock.MagicMock())
+        rqset = set(['FOOPAR1', 'FOOPAR2'])
+        fakearg = {'FOOPAR1': 'FOOVAL'}
+        mrq = p._missing_required_parameters(rqset, **fakearg)
+        assert mrq
+        assert mrq == ['FOOPAR2']
+
+    def test_missing_req_param_false(self):
+        p = PathElement(mock.MagicMock())
+        rqset = set(['FOOPAR1'])
+        fakearg = {'FOOPAR1': 'FOOVAL'}
+        mrq = p._missing_required_parameters(rqset, **fakearg)
+        assert not mrq
+
+    def test_check_load_parameters_fail(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['required_load_parameters'] = set(['FAKELOAD'])
+        with pytest.raises(MissingRequiredReadParameter) as RLPEIO:
+            p._check_load_parameters(FOOLOAD='FOOVAL')
+        assert "['FAKELOAD']" in RLPEIO.value.message
+
+    def test_check_create_parameters_fail(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['required_creation_parameters'] = set(['FAKECREATE'])
+        with pytest.raises(MissingRequiredCreationParameter) as RCPEIO:
+            p._check_create_parameters(FOOCREATE='FOOVAL')
+        assert "['FAKECREATE']" in RCPEIO.value.message
+
+    def test_check_command_parameters_fail(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['required_command_parameters'] = set(['FAKECOMMAND'])
+        with pytest.raises(MissingRequiredCommandParameter) as RCPEIO:
+            p._check_command_parameters(BARCOMMAND='FOOVAL')
+        assert "['FAKECOMMAND']" in RCPEIO.value.message
+
+    def test_check_exclusive_parameters_empty_attr(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['exclusive_attributes'] = []
+        fakearg = {'FOOEX': 'FOOVAL'}
+        # Check that any other exception is not thrown
+        p._check_exclusive_parameters(**fakearg)
+
+    def test_check_exclusive_parameters_pass(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['exclusive_attributes'] = [('FOOEX', 'BAREX')]
+        fakearg = {'FOOEX': 'FOOVAL'}
+        # Check ExclusiveAttributesPresent is not thrown
+        p._check_exclusive_parameters(**fakearg)
+
+    def test_check_exclusive_parameters_fail(self):
+        p = PathElement(mock.MagicMock())
+        p._meta_data['exclusive_attributes'] = [('FOOEX', 'BAREX')]
+        fakearg = {'FOOEX': 'FOOVAL', 'BAREX': 'BARVAL'}
+        with pytest.raises(ExclusiveAttributesPresent) as EAEIO:
+            p._check_exclusive_parameters(**fakearg)
+        assert 'FOOEX, BAREX' in EAEIO.value.message

f5/bigip/tm/sys/test/test_sys_application.py

@@ -279,13 +279,13 @@ def test_load_no_args(self, FakeService):
         with pytest.raises(MissingRequiredReadParameter) as ex:
             FakeService.load()
         assert ex.value.message == \
-            "Missing required params: set(['partition', 'name'])"
+            "Missing required params: ['partition', 'name']"
 
     def test_load_no_partition(self, FakeService):
         with pytest.raises(MissingRequiredReadParameter) as ex:
             FakeService.load(name='test_service')
         assert ex.value.message == \
-            "Missing required params: set(['partition'])"
+            "Missing required params: ['partition']"
 
 
 class TestServiceUpdate(object):

test/functional/tm/sys/test_failover.py

@@ -16,6 +16,7 @@
 from f5.bigip.tm.sys.failover import InvalidParameterValue
 from pprint import pprint as pp
 
+import time
 import pytest
 
 
@@ -68,9 +69,12 @@ def test_exec_cmd(self, bigip):
 
     def test_exec_cmd_cmdargs(self, bigip):
         f = bigip.sys.failover
-        f.exec_cmd('run', utilCmdArgs='offline persist')
+        f.exec_cmd('run', utilCmdArgs='offline')
         fl = bigip.sys.failover.load()
         assert 'Failover forced_offline' in fl.apiRawValues['apiAnonymous']
         f.exec_cmd('run', utilCmdArgs='online')
+        # We need this 1 sec delay as sometimes the status does not change
+        # straight away, causing the assertion to fail.
+        time.sleep(1)
         fl.refresh()
         assert 'Failover active' in fl.apiRawValues['apiAnonymous']