Merge "Remove convert_data function"

validations_libs/tests/test_utils.py

@@ -58,6 +58,34 @@ class TestUtils(TestCase):
         result = utils.parse_all_validations_on_disk('/foo/playbook')
         self.assertEqual(result, [fakes.FAKE_METADATA])
 
+    def test_parse_all_validations_on_disk_wrong_path_type(self):
+        self.assertRaises(TypeError,
+                          utils.parse_all_validations_on_disk,
+                          path=['/foo/playbook'])
+
+    def test_parse_all_validations_on_disk_wrong_groups_type(self):
+        self.assertRaises(TypeError,
+                          utils.parse_all_validations_on_disk,
+                          path='/foo/playbook',
+                          groups='foo1,foo2')
+
+    def test_get_validations_playbook_wrong_validation_id_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_playbook,
+                          path='/foo/playbook',
+                          validation_id='foo1,foo2')
+
+    def test_get_validations_playbook_wrong_groups_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_playbook,
+                          path='/foo/playbook',
+                          groups='foo1,foo2')
+
+    def test_get_validations_playbook_wrong_path_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_playbook,
+                          path=['/foo/playbook'])
+
     @mock.patch('os.path.isfile')
     @mock.patch('os.listdir')
     @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK)
@@ -67,23 +95,9 @@ class TestUtils(TestCase):
         mock_listdir.return_value = ['foo.yaml']
         mock_isfile.return_value = True
         result = utils.get_validations_playbook('/foo/playbook',
-                                                validation_id='foo')
+                                                validation_id=['foo'])
         self.assertEqual(result, ['/foo/playbook/foo.yaml'])
 
-    @mock.patch('os.path.isfile')
-    @mock.patch('os.listdir')
-    @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK)
-    @mock.patch('six.moves.builtins.open')
-    def test_get_validations_playbook_by_string_id(self, mock_open, mock_load,
-                                                   mock_listdir, mock_isfile):
-        validation_id = "foo,foo2,foo3"
-        mock_listdir.return_value = ['foo.yaml', 'foo2.yaml', 'foo3.yaml']
-        mock_isfile.return_value = True
-        result = utils.get_validations_playbook('/foo/playbook', validation_id)
-        self.assertEqual(result, ['/foo/playbook/foo.yaml',
-                                  '/foo/playbook/foo2.yaml',
-                                  '/foo/playbook/foo3.yaml'])
-
     @mock.patch('os.path.isfile')
     @mock.patch('os.listdir')
     @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK)
@@ -92,7 +106,7 @@ class TestUtils(TestCase):
                                                   mock_listdir, mock_isfile):
         mock_listdir.return_value = ['foo.yaml']
         mock_isfile.return_value = True
-        result = utils.get_validations_playbook('/foo/playbook', 'foo', 'prep')
+        result = utils.get_validations_playbook('/foo/playbook', ['foo'], ['prep'])
         self.assertEqual(result, ['/foo/playbook/foo.yaml',
                                   '/foo/playbook/foo.yaml'])
 
@@ -107,7 +121,7 @@ class TestUtils(TestCase):
         mock_listdir.return_value = ['foo.yaml']
         mock_isfile.return_value = True
         result = utils.get_validations_playbook('/foo/playbook',
-                                                'no_group')
+                                                groups=['no_group'])
         self.assertEqual(result, [])
 
     @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK)
@@ -149,12 +163,31 @@ class TestUtils(TestCase):
                           utils.get_validations_details,
                           validation=validation)
 
+    def test_get_validations_parameters_wrong_validations_data_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_parameters,
+                          validations_data='/foo/playbook1.yaml')
+
+    def test_get_validations_parameters_wrong_validation_name_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_parameters,
+                          validations_data=['/foo/playbook1.yaml',
+                                            '/foo/playbook2.yaml'],
+                          validation_name='playbook1,playbook2')
+
+    def test_get_validations_parameters_wrong_groups_type(self):
+        self.assertRaises(TypeError,
+                          utils.get_validations_parameters,
+                          validations_data=['/foo/playbook1.yaml',
+                                            '/foo/playbook2.yaml'],
+                          groups='group1,group2')
+
     @mock.patch('yaml.safe_load', return_value=fakes.FAKE_PLAYBOOK2)
     @mock.patch('six.moves.builtins.open')
     def test_get_validations_parameters_no_group(self, mock_open, mock_load):
 
         result = utils.get_validations_parameters(['/foo/playbook/foo.yaml'],
-                                                  'foo')
+                                                  ['foo'])
         output = {'foo': {'parameters': {'foo': 'bar'}}}
         self.assertEqual(result, output)
 
@@ -175,44 +208,6 @@ class TestUtils(TestCase):
                                                   [], [])
         self.assertEqual(result, {})
 
-    @mock.patch('six.moves.builtins.open')
-    def test_convert_data(self, mock_open):
-        data_string = "check-cpu,check-ram,check-disk-space"
-        data_list = ["check-cpu", "check-ram", "check-disk-space"]
-        result = utils.convert_data(data_string)
-        self.assertEqual(result, data_list)
-
-    @mock.patch('six.moves.builtins.open')
-    def test_convert_data_with_spaces(self, mock_open):
-        data_string = "check-cpu, check-ram , check-disk-space"
-        data_list = ["check-cpu", "check-ram", "check-disk-space"]
-        result = utils.convert_data(data_string)
-        self.assertEqual(result, data_list)
-
-    @mock.patch('six.moves.builtins.open')
-    def test_convert_data_with_comma_at_end(self, mock_open):
-        data_string = "check-cpu,"
-        data_list = ["check-cpu"]
-        result = utils.convert_data(data_string)
-        self.assertEqual(result, data_list)
-
-    @mock.patch('six.moves.builtins.open')
-    def test_convert_data_with_list(self, mock_open):
-        data_list = ["check-cpu", "check-ram", "check-disk-space"]
-        result = utils.convert_data(data_list)
-        self.assertEqual(result, data_list)
-
-    @mock.patch('six.moves.builtins.open')
-    def test_convert_data_with_non_list(self, mock_open):
-        data_dict = {
-            "val1": "check-cpu",
-            "val2": "check-ram",
-            "val3": "check-disk-space"
-        }
-        self.assertRaises(TypeError,
-                          utils.convert_data,
-                          data=data_dict)
-
     @mock.patch('validations_libs.utils.LOG', autospec=True)
     @mock.patch('validations_libs.utils.os.makedirs')
     @mock.patch(

validations_libs/utils.py

@@ -133,10 +133,9 @@ def parse_all_validations_on_disk(path, groups=None):
 
     :param path: The absolute path of the validations directory
     :type path: `string`
-    :param groups: Groups of validations. Could be a `list` or a
+    :param groups: Groups of validations
-                   comma-separated `string` of groups
+    :type groups: `list`
-    :type groups: `list` or `string`
+    :return: A list of validations metadata
-    :return: A list of validations metadata.
     :rtype: `list`
 
     :Example:
@@ -152,13 +151,16 @@ def parse_all_validations_on_disk(path, groups=None):
       'id': 'check-cpu',
       'name': 'Verify if the server fits the CPU core requirements'}]
     """
+    if not isinstance(path, six.string_types):
+        raise TypeError("The 'path' argument should be a String")
 
-    results = []
     if not groups:
         groups = []
-    else:
-        groups = convert_data(groups)
 
+    if not isinstance(groups, list):
+        raise TypeError("The 'groups' argument should be a List")
+
+    results = []
     validations_abspath = glob.glob("{path}/*.yaml".format(path=path))
 
     LOG.debug(
@@ -183,9 +185,9 @@ def get_validations_playbook(path, validation_id=None, groups=None):
     :param path: Path of the validations playbooks
     :type path: `string`
     :param validation_id: List of validation name
-    :type validation_id: `list` or a `string` of comma-separated validations
+    :type validation_id: `list`
     :param groups: List of validation group
-    :type groups: `list` or a `string` of comma-separated groups
+    :type groups: `list`
     :return: A list of absolute validations playbooks path
     :rtype: `list`
 
@@ -198,15 +200,20 @@ def get_validations_playbook(path, validation_id=None, groups=None):
     ['/usr/share/ansible/validation-playbooks/512e.yaml',
      '/usr/share/ansible/validation-playbooks/check-cpu.yaml',]
     """
+    if not isinstance(path, six.string_types):
+        raise TypeError("The 'path' argument should be a String")
+
     if not validation_id:
         validation_id = []
-    else:
+
-        validation_id = convert_data(validation_id)
+    if not isinstance(validation_id, list):
+        raise TypeError("The 'validation_id' argument should be a List")
 
     if not groups:
         groups = []
-    else:
+
-        groups = convert_data(groups)
+    if not isinstance(groups, list):
+        raise TypeError("The 'groups' argument should be a List")
 
     pl = []
     for f in os.listdir(path):
@@ -287,7 +294,7 @@ def get_validations_details(validation):
      'name': 'Verify the server fits the something requirements'}
     """
     if not isinstance(validation, six.string_types):
-        raise TypeError("The input data should be a String")
+        raise TypeError("The 'validation' argument should be a String")
 
     results = parse_all_validations_on_disk(constants.ANSIBLE_VALIDATION_DIR)
     for r in results:
@@ -321,7 +328,7 @@ def get_validations_data(validation, path=constants.ANSIBLE_VALIDATION_DIR):
      'Parameters': {'param1': 24}}
     """
     if not isinstance(validation, six.string_types):
-        raise TypeError("The input data should be a String")
+        raise TypeError("The 'validation' argument should be a String")
 
     data = {}
     val_path = "{}/{}.yaml".format(path, validation)
@@ -339,8 +346,9 @@ def get_validations_data(validation, path=constants.ANSIBLE_VALIDATION_DIR):
     return data
 
 
-def get_validations_parameters(validations_data, validation_name=[],
+def get_validations_parameters(validations_data,
-                               groups=[]):
+                               validation_name=None,
+                               groups=None):
     """Return parameters for a list of validations
 
 
@@ -363,6 +371,21 @@ def get_validations_parameters(validations_data, validation_name=[],
     {'check-cpu': {'parameters': {'minimal_cpu_count': 8}},
      'check-ram': {'parameters': {'minimal_ram_gb': 24}}}
     """
+    if not isinstance(validations_data, list):
+        raise TypeError("The 'validations_data' argument should be a List")
+
+    if not validation_name:
+        validation_name = []
+
+    if not isinstance(validation_name, list):
+        raise TypeError("The 'validation_name' argument should be a List")
+
+    if not groups:
+        groups = []
+
+    if not isinstance(groups, list):
+        raise TypeError("The 'groups' argument should be a List")
+
     params = {}
     for val in validations_data:
         v = Validation(val)
@@ -372,41 +395,3 @@ def get_validations_parameters(validations_data, validation_name=[],
             }
 
     return params
-
-
-def convert_data(data=''):
-    """Transform a string containing comma-separated validation or group name
-    into a list. If `data` is already a list, it will simply return `data`.
-
-    :param data: A string or a list
-    :type data: `string` or `list`
-    :return: A list of data
-    :rtype: `list`
-    :raises: a `TypeError` exception if `data` is not a list or a string
-
-    :Example:
-
-    >>> data = "check-cpu,check-ram,check-disk-space"
-    >>> convert_data(data)
-    ['check-cpu', 'check-ram', 'check-disk-space']
-    ...
-    >>> data = "check-cpu , check-ram , check-disk-space"
-    >>> convert_data(data)
-    ['check-cpu', 'check-ram', 'check-disk-space']
-    ...
-    >>> data = "check-cpu,"
-    >>> convert_data(data)
-    ['check-cpu']
-    ...
-    >>> data = ['check-cpu', 'check-ram', 'check-disk-space']
-    >>> convert_data(data)
-    ['check-cpu', 'check-ram', 'check-disk-space']
-    """
-    if isinstance(data, six.string_types):
-        return [
-            conv_data.strip() for conv_data in data.split(',') if conv_data
-        ]
-    elif not isinstance(data, list):
-        raise TypeError("The input data should be either a List or a String")
-    else:
-        return data

validations_libs/validation_actions.py

@@ -343,8 +343,6 @@ class ValidationActions(object):
             except Exception as e:
                 raise(e)
         elif validation_name:
-            validation_name = v_utils.convert_data(validation_name)
-
             playbooks = v_utils.get_validations_playbook(validations_dir,
                                                          validation_name,
                                                          group)