Validation actions refactor

Unused workdir argument of the run_validations method removed.

Host and playbook skipping simplified.

Docstrings adjusted, including some of the more important
inline code examples.

Test sensitivity to API changes was increased.
More strict assertions have been put on the calls made in CLI.

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Change-Id: I781f6a6f9fc4bd558af56b648f0e0ee9f165dfab

validations_libs/cli/show.py

@@ -69,7 +69,7 @@ class ShowGroup(Lister):
         """Take validation action"""
 
         v_actions = ValidationActions(parsed_args.validation_dir)
-        return v_actions.group_information(constants.VALIDATION_GROUPS_INFO)
+        return v_actions.group_information()
 
 
 class ShowParameter(ShowOne):

validations_libs/group.py

@@ -68,10 +68,10 @@ class Group(object):
         return self.data
 
     @property
-    def get_formated_group(self):
+    def get_formated_groups(self):
-        """Get a formated content for output display
+        """Get a formated list of groups for output display
 
-        :return:
+        :return: information about parsed groups
         :rtype: `list` of `tuples`
 
         :Example:

validations_libs/tests/cli/test_history.py

@@ -29,7 +29,8 @@ class TestListHistory(BaseCommand):
         self.cmd = history.ListHistory(self.app, None)
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
-                'show_history')
+                'show_history',
+                autospec=True)
     def test_list_history(self, mock_history):
         arglist = ['--validation-log-dir', '/foo/log/dir']
         verifylist = [('validation_log_dir', '/foo/log/dir')]
@@ -53,7 +54,8 @@ class TestGetHistory(BaseCommand):
 
     @mock.patch('validations_libs.validation_logs.ValidationLogs.'
                 'get_logfile_content_by_uuid',
-                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST)
+                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST,
+                autospec=True)
     def test_get_history(self, mock_logs):
         arglist = ['123']
         verifylist = [('uuid', '123')]
@@ -63,7 +65,8 @@ class TestGetHistory(BaseCommand):
 
     @mock.patch('validations_libs.validation_logs.ValidationLogs.'
                 'get_logfile_content_by_uuid',
-                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST)
+                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST,
+                autospec=True)
     def test_get_history_from_log_dir(self, mock_logs):
         arglist = ['123', '--validation-log-dir', '/foo/log/dir']
         verifylist = [('uuid', '123'), ('validation_log_dir', '/foo/log/dir')]
@@ -73,7 +76,8 @@ class TestGetHistory(BaseCommand):
 
     @mock.patch('validations_libs.validation_logs.ValidationLogs.'
                 'get_logfile_content_by_uuid',
-                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST)
+                return_value=fakes.VALIDATIONS_LOGS_CONTENTS_LIST,
+                autospec=True)
     def test_get_history_full_arg(self, mock_logs):
         arglist = ['123', '--full']
         verifylist = [('uuid', '123'), ('full', True)]

validations_libs/tests/cli/test_list.py

@@ -30,14 +30,15 @@ class TestList(BaseCommand):
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'list_validations',
-                return_value=fakes.VALIDATIONS_LIST)
+                return_value=fakes.VALIDATIONS_LIST,
+                autospec=True)
     def test_list_validations(self, mock_list):
         arglist = ['--validation-dir', 'foo']
         verifylist = [('validation_dir', 'foo')]
 
         val_list = [
             {'description': 'My Validation One Description',
-             'groups': ['prep', 'pre-deployment'],
+             'groups': ['prep', 'pre-deployment', 'no-op', 'post'],
              'categories': ['os', 'system', 'ram'],
              'products': ['product1'],
              'id': 'my_val1',
@@ -45,7 +46,7 @@ class TestList(BaseCommand):
              'parameters': {}
             }, {
              'description': 'My Validation Two Description',
-             'groups': ['prep', 'pre-introspection'],
+             'groups': ['prep', 'pre-introspection', 'post', 'pre'],
              'categories': ['networking'],
              'products': ['product1'],
              'id': 'my_val2',
@@ -59,7 +60,8 @@ class TestList(BaseCommand):
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'list_validations',
-                return_value=[])
+                return_value=[],
+                autospec=True)
     def test_list_validations_empty(self, mock_list):
         arglist = ['--validation-dir', 'foo']
         verifylist = [('validation_dir', 'foo')]
@@ -69,7 +71,8 @@ class TestList(BaseCommand):
         self.assertEqual(result, [])
 
     @mock.patch('validations_libs.utils.parse_all_validations_on_disk',
-                return_value=fakes.VALIDATIONS_LIST_GROUP)
+                return_value=fakes.VALIDATIONS_LIST_GROUP,
+                autospec=True)
     def test_list_validations_group(self, mock_list):
         arglist = ['--validation-dir', 'foo', '--group', 'prep']
         verifylist = [('validation_dir', 'foo'),
@@ -82,7 +85,8 @@ class TestList(BaseCommand):
         self.assertEqual(result, val_list)
 
     @mock.patch('validations_libs.utils.parse_all_validations_on_disk',
-                return_value=fakes.VALIDATIONS_LIST_GROUP)
+                return_value=fakes.VALIDATIONS_LIST_GROUP,
+                autospec=True)
     def test_list_validations_by_category(self, mock_list):
         arglist = ['--validation-dir', 'foo', '--category', 'networking']
         verifylist = [('validation_dir', 'foo'),
@@ -95,7 +99,8 @@ class TestList(BaseCommand):
         self.assertEqual(result, val_list)
 
     @mock.patch('validations_libs.utils.parse_all_validations_on_disk',
-                return_value=fakes.VALIDATIONS_LIST_GROUP)
+                return_value=fakes.VALIDATIONS_LIST_GROUP,
+                autospec=True)
     def test_list_validations_by_product(self, mock_list):
         arglist = ['--validation-dir', 'foo', '--product', 'product1']
         verifylist = [('validation_dir', 'foo'),

validations_libs/tests/cli/test_run.py

@@ -32,7 +32,8 @@ class TestRun(BaseCommand):
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=None)
+                return_value=None,
+                autospec=True)
     def test_run_command_return_none(self, mock_run):
         arglist = ['--validation', 'foo']
         verifylist = [('validation_name', ['foo'])]
@@ -42,7 +43,8 @@ class TestRun(BaseCommand):
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_success(self, mock_run):
         arglist = ['--validation', 'foo']
         verifylist = [('validation_name', ['foo'])]
@@ -63,7 +65,8 @@ class TestRun(BaseCommand):
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_vars(self, mock_run, mock_user, mock_print,
                                     mock_log_dir):
         run_called_args = {
@@ -89,7 +92,9 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
     @mock.patch('validations_libs.cli.common.print_dict')
@@ -97,7 +102,8 @@ class TestRun(BaseCommand):
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_vars_twice(self, mock_run, mock_user,
                                           mock_print, mock_log_dir):
         run_called_args = {
@@ -124,7 +130,9 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     def test_run_command_exclusive_vars(self):
         arglist = ['--validation', 'foo',
@@ -143,7 +151,8 @@ class TestRun(BaseCommand):
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_vars_file(self, mock_run, mock_user, mock_open,
                                          mock_yaml, mock_log_dir):
 
@@ -170,14 +179,17 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
     @mock.patch('getpass.getuser',
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_env_vars(self, mock_run, mock_user, mock_log_dir):
         run_called_args = {
             'inventory': 'localhost',
@@ -202,14 +214,17 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
     @mock.patch('getpass.getuser',
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_env_vars_with_custom_callback(self,
                                                              mock_run,
                                                              mock_user,
@@ -238,14 +253,17 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
     @mock.patch('getpass.getuser',
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_env_vars_twice(self, mock_run, mock_user, mock_log_dir):
         run_called_args = {
             'inventory': 'localhost',
@@ -271,14 +289,17 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
     @mock.patch('getpass.getuser',
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_SUCCESS_RUN),
+                autospec=True)
     def test_run_command_extra_env_vars_and_extra_vars(self,
                                                        mock_run,
                                                        mock_user,
@@ -308,7 +329,9 @@ class TestRun(BaseCommand):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
-        mock_run.assert_called_with(**run_called_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
 
     def test_run_command_exclusive_wrong_extra_vars(self):
         arglist = ['--validation', 'foo',
@@ -324,36 +347,9 @@ class TestRun(BaseCommand):
                 return_value='doe')
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
                 'run_validations',
-                return_value=copy.deepcopy(fakes.FAKE_FAILED_RUN))
+                return_value=copy.deepcopy(fakes.FAKE_FAILED_RUN),
+                autospec=True)
     def test_run_command_failed_validation(self, mock_run, mock_user, mock_log_dir):
-        run_called_args = {
-            'inventory': 'localhost',
-            'limit_hosts': None,
-            'group': [],
-            'category': [],
-            'product': [],
-            'extra_vars': None,
-            'validations_dir': '/usr/share/ansible/validation-playbooks',
-            'base_dir': '/usr/share/ansible',
-            'validation_name': ['foo'],
-            'extra_env_vars': None,
-            'python_interpreter': sys.executable,
-            'quiet': True,
-            'ssh_user': 'doe',
-            'log_path': mock_log_dir}
-
-        arglist = ['--validation', 'foo']
-        verifylist = [('validation_name', ['foo'])]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        self.assertRaises(RuntimeError, self.cmd.take_action, parsed_args)
-        mock_run.assert_called_with(**run_called_args)
-
-    @mock.patch('getpass.getuser',
-                return_value='doe')
-    @mock.patch('validations_libs.validation_actions.ValidationActions.'
-                'run_validations',
-                return_value=[])
-    def test_run_command_no_validation(self, mock_run, mock_user):
         run_called_args = {
             'inventory': 'localhost',
             'limit_hosts': None,
@@ -367,10 +363,59 @@ class TestRun(BaseCommand):
             'extra_env_vars': {'key2': 'value2'},
             'python_interpreter': sys.executable,
             'quiet': True,
-            'ssh_user': 'doe'}
+            'ssh_user': 'doe',
+            'log_path': mock_log_dir}
 
-        arglist = ['--validation', 'foo']
+        arglist = [
-        verifylist = [('validation_name', ['foo'])]
+            '--validation', 'foo',
+            '--extra-vars', 'key=value',
+            '--extra-env-vars', 'key2=value2']
+        verifylist = [
+            ('validation_name', ['foo']),
+            ('extra_vars', {'key': 'value'}),
+            ('extra_env_vars', {'key2': 'value2'})]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.assertRaises(RuntimeError, self.cmd.take_action, parsed_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)
+
+    @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')
+    @mock.patch('getpass.getuser',
+                return_value='doe')
+    @mock.patch('validations_libs.validation_actions.ValidationActions.'
+                'run_validations',
+                return_value=[],
+                autospec=True)
+    def test_run_command_no_validation(self, mock_run, mock_user, mock_log_dir):
+        run_called_args = {
+            'inventory': 'localhost',
+            'limit_hosts': None,
+            'group': [],
+            'category': [],
+            'product': [],
+            'extra_vars': {'key': 'value'},
+            'validations_dir': '/usr/share/ansible/validation-playbooks',
+            'base_dir': '/usr/share/ansible',
+            'validation_name': ['foo'],
+            'extra_env_vars': {'key2': 'value2'},
+            'python_interpreter': sys.executable,
+            'quiet': True,
+            'ssh_user': 'doe',
+            'log_path': mock_log_dir}
+
+        arglist = [
+            '--validation', 'foo',
+            '--extra-vars', 'key=value',
+            '--extra-env-vars', 'key2=value2']
+        verifylist = [
+            ('validation_name', ['foo']),
+            ('extra_vars', {'key': 'value'}),
+            ('extra_env_vars', {'key2': 'value2'})]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.assertRaises(RuntimeError, self.cmd.take_action, parsed_args)
+        call_args = mock_run.mock_calls[0][2]
+
+        self.assertDictEqual(call_args, run_called_args)

validations_libs/tests/cli/test_show.py

@@ -17,6 +17,8 @@ try:
 except ImportError:
     import mock
 
+from validations_libs import group
+from validations_libs.validation_actions import ValidationActions
 from validations_libs.cli import show
 from validations_libs.tests import fakes
 from validations_libs.tests.cli.fakes import BaseCommand
@@ -47,22 +49,18 @@ class TestShowGroup(BaseCommand):
     @mock.patch('yaml.safe_load', return_value=fakes.GROUP)
     @mock.patch('six.moves.builtins.open')
     def test_show_validations_group_info(self, mock_open, mock_yaml, mock_actions):
-        arglist = []
 
-        mock_info = mock.MagicMock()
+        method_calls = [
-        mock_info.group_information = mock.MagicMock(return_value='foo')
+            mock.call(fakes.FAKE_VALIDATIONS_PATH),
-        mock_actions.return_value = mock_info
+            mock.call().group_information()]
+
+        arglist = []
 
         parsed_args = self.check_parser(self.cmd, arglist, [])
 
-        group_info = self.cmd.take_action(parsed_args)
+        self.cmd.take_action(parsed_args)
 
-        mock_actions.assert_called_once_with(
+        mock_actions.assert_has_calls(method_calls)
-            validation_path=fakes.FAKE_VALIDATIONS_PATH)
-
-        mock_info.group_information.assert_called_once()
-
-        self.assertEqual('foo', group_info)
 
 
 class TestShowParameter(BaseCommand):
@@ -72,10 +70,15 @@ class TestShowParameter(BaseCommand):
         self.cmd = show.ShowParameter(self.app, None)
 
     @mock.patch('six.moves.builtins.open')
-    def test_show_validations_parameters_by_group(self, mock_open):
+    @mock.patch('validations_libs.validation_actions.ValidationActions.'
+                'show_validations_parameters', autospec=True)
+    def test_show_validations_parameters_by_group(self, mock_show, mock_open):
         arglist = ['--group', 'prep']
         verifylist = [('group', ['prep'])]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+
+        mock_show.assert_called_once()
 
     def test_show_parameter_exclusive_group(self):
         arglist = ['--validation', 'foo', '--group', 'bar']
@@ -85,23 +88,32 @@ class TestShowParameter(BaseCommand):
                           arglist, verifylist)
 
     @mock.patch('six.moves.builtins.open')
-    def test_show_validations_parameters_by_validations(self, mock_open):
+    @mock.patch('validations_libs.validation_actions.ValidationActions.'
+                'show_validations_parameters', autospec=True)
+    def test_show_validations_parameters_by_validations(self, mock_show, mock_open):
         arglist = ['--group', 'prep']
         verifylist = [('group', ['prep'])]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.cmd.take_action(parsed_args)
+
+        mock_show.assert_called_once()
 
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
-                'show_validations_parameters')
+                'show_validations_parameters', autospec=True)
     def test_show_validations_parameters_by_categories(self, mock_show):
         arglist = ['--category', 'os']
         verifylist = [('category', ['os'])]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
 
+        mock_show.assert_called_once()
+
     @mock.patch('validations_libs.validation_actions.ValidationActions.'
-                'show_validations_parameters')
+                'show_validations_parameters', autospec=True)
     def test_show_validations_parameters_by_products(self, mock_show):
         arglist = ['--product', 'product1']
         verifylist = [('product', ['product1'])]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
+
+        mock_show.assert_called_once()

validations_libs/tests/fakes.py

@@ -17,7 +17,7 @@ from validations_libs import constants
 
 VALIDATIONS_LIST = [{
     'description': 'My Validation One Description',
-    'groups': ['prep', 'pre-deployment'],
+    'groups': ['prep', 'pre-deployment', 'no-op', 'post'],
     'categories': ['os', 'system', 'ram'],
     'products': ['product1'],
     'id': 'my_val1',
@@ -25,7 +25,7 @@ VALIDATIONS_LIST = [{
     'parameters': {}
 }, {
     'description': 'My Validation Two Description',
-    'groups': ['prep', 'pre-introspection'],
+    'groups': ['prep', 'pre-introspection', 'post', 'pre'],
     'categories': ['networking'],
     'products': ['product1'],
     'id': 'my_val2',

validations_libs/tests/test_group.py

@@ -40,7 +40,7 @@ class TestGroup(TestCase):
     def test_get_formated_group(self, mock_open, mock_yaml):
         grp = Group('/tmp/foo')
         ret = [('no-op', 'noop-foo'), ('post', 'post-foo'), ('pre', 'pre-foo')]
-        data = grp.get_formated_group
+        data = grp.get_formated_groups
         self.assertEqual(data, ret)
 
     @mock.patch('yaml.safe_load', return_value=fakes.GROUP)

validations_libs/tests/test_validation_actions.py

@@ -40,12 +40,12 @@ class TestValidationActions(TestCase):
         self.assertEqual(validations_list.list_validations(),
                          (self.column_name, [('my_val1',
                                               'My Validation One Name',
-                                              ['prep', 'pre-deployment'],
+                                              ['prep', 'pre-deployment', 'no-op', 'post'],
                                               ['os', 'system', 'ram'],
                                               ['product1']),
                                              ('my_val2',
                                               'My Validation Two Name',
-                                              ['prep', 'pre-introspection'],
+                                              ['prep', 'pre-introspection', 'post', 'pre'],
                                               ['networking'],
                                               ['product1'])]))
 
@@ -228,9 +228,8 @@ class TestValidationActions(TestCase):
 
         run = ValidationActions()
         self.assertRaises(RuntimeError, run.run_validations,
-                          validation_name='fake.yaml',
+                          validation_name=['fake'],
-                          validations_dir='/tmp/foo'
+                          validations_dir='/tmp/foo')
-                          )
 
     @mock.patch('validations_libs.utils.os.makedirs')
     @mock.patch('validations_libs.utils.os.access', return_value=True)
@@ -377,12 +376,12 @@ class TestValidationActions(TestCase):
     @mock.patch('six.moves.builtins.open')
     def test_group_information(self, mock_open, mock_yaml, mock_data):
         v_actions = ValidationActions()
-        col, values = v_actions.group_information('512e')
+        col, values = v_actions.group_information()
         self.assertEqual(col, ('Groups', 'Description',
                                'Number of Validations'))
-        self.assertEqual(values, [('no-op', 'noop-foo', 2),
+        self.assertEqual(values, [('no-op', 'noop-foo', 1),
                                   ('post', 'post-foo', 2),
-                                  ('pre', 'pre-foo', 2)])
+                                  ('pre', 'pre-foo', 1)])
 
     @mock.patch('six.moves.builtins.open')
     def test_show_validations_parameters_wrong_validations_type(self, mock_open):

validations_libs/validation_actions.py

@@ -42,11 +42,14 @@ class ValidationActions(object):
 
     """
 
-    def __init__(self, validation_path=constants.ANSIBLE_VALIDATION_DIR):
+    def __init__(self, validation_path=constants.ANSIBLE_VALIDATION_DIR,
+                 groups_path=constants.VALIDATION_GROUPS_INFO):
 
         self.log = logging.getLogger(__name__ + ".ValidationActions")
         self.validation_path = validation_path
 
+        self.groups_path = groups_path
+
     def list_validations(self,
                          groups=None,
                          categories=None,
@@ -79,7 +82,7 @@ class ValidationActions(object):
             | val3 | val_name3 | ['group4']           | ['category3'] | ['product3'] |
             +------+-----------+----------------------+---------------+--------------+
 
-        :Example:
+        :example:
 
         >>> path = "/foo/bar"
         >>> groups = ['group1']
@@ -106,8 +109,7 @@ class ValidationActions(object):
             path=self.validation_path,
             groups=groups,
             categories=categories,
-            products=products
+            products=products)
-        )
 
         self.log.debug(
             "Parsed {} validations.".format(len(validations))
@@ -135,7 +137,7 @@ class ValidationActions(object):
         :return: The detailed information for a validation
         :rtype: `dict`
 
-        :Example:
+        :example:
 
         >>> path = "/foo/bar"
         >>> validation = 'foo'
@@ -167,28 +169,28 @@ class ValidationActions(object):
         data.update(data_format)
         return data
 
-    def _skip_hosts(self, skip_list, playbook, limit_hosts=None):
+    def _skip_hosts(self, validation, limit_hosts=None):
         """Check Ansible Hosts and return an updated limit_hosts
-        :param skip_list: The list of the validation to skip
+        :param validation: parsed validation playbook
-        :type validation_name: ``dict``
+        :type validation: `dict`
-        :param playbook: The name of the playbook
-        :type base_dir: ``string``
         :param limit_hosts: Limit the execution to the hosts.
         :type limit_hosts: ``string``
 
         :return the limit hosts according the skip_list or None if the
                 validation should be skipped on ALL hosts.
-        :example
+        :example:
-            limit_hosts = 'cloud1,cloud2'
-            skip_list = {'xyz': {'hosts': 'cloud1',
-                                          'reason': None,
-                                          'lp': None}
-                        }
-            >>> _skip_hosts(skip_list, playbook, limit_hosts='cloud1,cloud2')
-            'cloud2,!cloud1'
 
+            >>> v_actions = ValidationActions()
+            >>> limit_hosts = 'cloud1,cloud2'
+            >>> validation = {
+            ...     'hosts': 'cloud1',
+            ...     'reason': None,
+            ...     'lp': None}
+            >>> v_actions._skip_hosts(validation, limit_hosts='cloud1,cloud2')
+            '!cloud1,cloud2'
         """
-        hosts = skip_list[playbook].get('hosts')
+
+        hosts = validation.get('hosts')
         if hosts == 'ALL' or hosts is None:
             return None
         else:
@@ -201,40 +203,49 @@ class ValidationActions(object):
 
     def _skip_playbook(self, skip_list, playbook, limit_hosts=None):
         """Check if playbook is in the ski plist
-        :param skip_list: The list of the validation to skip
+        :param skip_list: Dictionary of validations to skip.
-        :type validation_name: ``dict``
+        :type skip_list: `dictionary`
         :param playbook: The name of the playbook
-        :type base_dir: ``string``
+        :type playbook: `string`
         :param limit_hosts: Limit the execution to the hosts.
         :type limit_hosts: ``string``
 
         :return a tuple of playbook and hosts
-        :example
+        :rtype: `tuple`
-            skip_list = {'xyz': {'hosts': 'cloud1',
+
-                                          'reason': None,
+        :example:
-                                          'lp': None}
+            >>> skip_list = {
-                        }
+            ... 'xyz': {
+            ...     'hosts': 'cloud1',
+            ...     'reason': None,
+            ...     'lp': None}}
+
             If playbook not in skip list:
-            >>> _skip_playbook(skip_list, 'foo', None)
+            >>> v_actions = ValidationActions()
+            >>> v_actions._skip_playbook(skip_list, 'foo', None)
             ('foo', None)
 
             If playbook in the skip list, but with restriction only on
             host cloud1:
-            >>> _skip_playbook(skip_list, 'xyz', None)
+            >>> v_actions = ValidationActions()
+            >>> v_actions._skip_playbook(skip_list, 'xyz', None)
             ('xyz', '!cloud1')
 
             If playbook in the skip list, and should be skip on ALL hosts:
-            skip_list = {'xyz': {'hosts': 'ALL',
+            >>> skip_list = {
-                                 'reason': None,
+            ... 'xyz': {
-                                 'lp': None}
+            ...     'hosts': 'ALL',
-                        }
+            ...     'reason': None,
-            >>> _skip_playbook(skip_list, 'xyz', None)
+            ...     'lp': None}}
+            >>> v_actions = ValidationActions()
+            >>> v_actions._skip_playbook(skip_list, 'xyz', None)
             (None, None)
 
         """
         if skip_list:
-            if playbook in skip_list.keys():
+            if playbook in skip_list:
-                _hosts = self._skip_hosts(skip_list, playbook,
+                _hosts = self._skip_hosts(
+                    skip_list[playbook],
                     limit_hosts)
                 if _hosts:
                     return playbook, _hosts
@@ -248,6 +259,14 @@ class ValidationActions(object):
         with the last time the file was modified serving as a key.
         Finally we take the last `n` logs, where `n` == `history_limit`
         and return them while discarding the time information.
+
+        :param logs: List of validation log file paths
+        :type logs: `list`
+        :param history_limit: number of entries to display
+        :type history_limit: `int`
+
+        :return: List of time-modified, path tuples of length =< history_limit
+        :rtype: `list`
         """
 
         history_limit = min(history_limit, len(logs))
@@ -262,7 +281,7 @@ class ValidationActions(object):
                         group=None, category=None, product=None,
                         extra_vars=None, validations_dir=None,
                         extra_env_vars=None, ansible_cfg=None, quiet=True,
-                        workdir=None, limit_hosts=None, run_async=False,
+                        limit_hosts=None, run_async=False,
                         base_dir=constants.DEFAULT_VALIDATIONS_BASEDIR,
                         log_path=constants.VALIDATIONS_LOG_BASEDIR,
                         python_interpreter=None, skip_list=None,
@@ -271,7 +290,7 @@ class ValidationActions(object):
         """Run one or multiple validations by name(s), by group(s) or by
         product(s)
 
-        :param validation_name: A list of validation names
+        :param validation_name: A list of validation names.
         :type validation_name: ``list``
         :param inventory: Either proper inventory file, or a comma-separated
                           list. (Defaults to ``localhost``)
@@ -296,8 +315,6 @@ class ValidationActions(object):
         :type ansible_cfg: ``string``
         :param quiet: Disable all output (Defaults to ``True``)
         :type quiet: ``Boolean``
-        :param workdir: Location of the working directory
-        :type workdir: ``string``
         :param limit_hosts: Limit the execution to the hosts.
         :type limit_hosts: ``string``
         :param run_async: Enable the Ansible asynchronous mode
@@ -337,7 +354,7 @@ class ValidationActions(object):
         :param ssh_user: Ssh user for Ansible remote connection
         :type ssh_user: ``string``
 
-        :Example:
+        :example:
 
         >>> path = "/u/s/a"
         >>> validation_name = ['foo', 'bar']
@@ -374,8 +391,8 @@ class ValidationActions(object):
             )
             validations = v_utils.parse_all_validations_on_disk(
                 path=validations_dir, groups=group,
-                categories=category, products=product
+                categories=category, products=product)
-            )
+
             for val in validations:
                 playbooks.append(val.get('id') + '.yaml')
         elif validation_name:
@@ -383,14 +400,17 @@ class ValidationActions(object):
                                                          validation_name)
 
             if not playbooks or len(validation_name) != len(playbooks):
-                p = []
+                found_playbooks = []
                 for play in playbooks:
-                    p.append(os.path.basename(os.path.splitext(play)[0]))
+                    found_playbooks.append(
+                        os.path.basename(os.path.splitext(play)[0]))
 
-                unknown_validation = list(set(validation_name) - set(p))
+                unknown_validations = list(
+                    set(validation_name) - set(found_playbooks))
 
-                msg = "Validation {} not found in {}.".format(
+                msg = (
-                    unknown_validation, validations_dir)
+                    "Following validations were not found in '{}': {}").format(
+                    validations_dir, ','.join(unknown_validations))
 
                 raise RuntimeError(msg)
         else:
@@ -468,7 +488,7 @@ class ValidationActions(object):
         vlog = ValidationLogs(log_path)
         return vlog.get_results(uuid)
 
-    def group_information(self, groups):
+    def group_information(self):
         """Get Information about Validation Groups
 
         This is used to print table from python ``Tuple`` with ``PrettyTable``.
@@ -483,33 +503,41 @@ class ValidationActions(object):
             | group3   | Description of group3    |                     1 |
             +----------+--------------------------+-----------------------+
 
-        :param groups: The absolute path of the groups.yaml file
-        :type groups: ``string``
-
         :return: The list of the available groups with their description and
                  the numbers of validation belonging to them.
         :rtype: ``tuple``
 
-        :Example:
+        :example:
 
         >>> groups = "/foo/bar/groups.yaml"
-        >>> actions = ValidationActions(constants.ANSIBLE_VALIDATION_DIR)
+        >>> actions = ValidationActions(constants.ANSIBLE_VALIDATION_DIR, groups)
-        >>> group_info = actions.group_information(groups)
+        >>> group_info = actions.group_information()
         >>> print(group_info)
         (('Groups', 'Desciption', 'Number of Validations'),
          [('group1', 'Description of group1', 3),
           ('group2', 'Description of group2', 12),
           ('group3', 'Description of group3', 1)])
         """
-        val_gp = Group(groups)
+
-        group = val_gp.get_formated_group
+        val_group = Group(self.groups_path)
+        group_definitions = val_group.get_formated_groups
 
         group_info = []
-        # Get validations number by groups
+
-        for gp in group:
         validations = v_utils.parse_all_validations_on_disk(
-                self.validation_path, gp[0])
+                path=self.validation_path,
-            group_info.append((gp[0], gp[1], len(validations)))
+                groups=[group[0] for group in group_definitions])
+
+        # Get validations number by group
+        for group in group_definitions:
+            n_matches = len(
+                [val for val in validations if group[0] in val['groups']])
+
+            group_info.append((
+                group[0],
+                group[1],
+                n_matches))
+
         column_name = ("Groups", "Description", "Number of Validations")
         return (column_name, group_info)
 
@@ -567,6 +595,7 @@ class ValidationActions(object):
                 }
             }
         }
+
         """
         if not validations:
             validations = []
@@ -662,7 +691,7 @@ class ValidationActions(object):
                  history
         :rtype: ``tuple``
 
-        :Example:
+        :example:
 
         >>> actions = ValidationActions(constants.ANSIBLE_VALIDATION_DIR)
         >>> print(actions.show_history())
@@ -700,6 +729,7 @@ class ValidationActions(object):
          'PASSED',
          '2020-11-13T11:47:50.279662Z',
          '0:00:02.237')])
+
         """
         vlogs = ValidationLogs(log_path)
         if validation_ids:
@@ -746,7 +776,7 @@ class ValidationActions(object):
         :return: A list of validations execution with details and by status
         :rtype: ``tuple``
 
-        :Example:
+        :example:
 
         >>> actions = ValidationActions(validation_path='/foo/bar')
         >>> status = actions.get_status(validation_id='foo'))