Merge pull request #3563 from rhcarvalho/misc-cleanup

Misc cleanup

9 files changed, 48 insertions, 140 deletions

diff --git a/roles/lib_utils/src/test/unit/test_yedit.py b/roles/lib_utils/src/test/unit/test_yedit.py
index ce5e027a7..a22cdee56 100755
--- a/roles/lib_utils/src/test/unit/test_yedit.py
+++ b/roles/lib_utils/src/test/unit/test_yedit.py
@@ -200,8 +200,6 @@ class YeditTest(unittest.TestCase):
         yed.append('x:y:z', [5, 6])
         yed.append('x:y:z', [5, 6])
         self.assertTrue(yed.get('x:y:z') == [1, 2, 3, [5, 6], [5, 6]])
-        # pylint: disable=maybe-no-member
-        self.assertTrue(2 == yed.get('x:y:z').count([5, 6]))
         self.assertFalse(yed.exists('x:y:z', 4))
 
     def test_add_item_to_dict(self):
diff --git a/roles/openshift_master_facts/test/conftest.py b/roles/openshift_master_facts/test/conftest.py
index e67d24f04..140cced73 100644
--- a/roles/openshift_master_facts/test/conftest.py
+++ b/roles/openshift_master_facts/test/conftest.py
@@ -20,7 +20,7 @@ def priorities_lookup():
 
 
 @pytest.fixture()
-def facts(request):
+def facts():
     return {
         'openshift': {
             'common': {}
diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
index 5a9e545a3..1fab84c71 100644
--- a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
+++ b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
@@ -131,7 +131,9 @@ def short_version_fixture(request, facts):
 
 def test_short_version_kwarg(predicates_lookup, short_version_kwarg_fixture, regions_enabled):
     facts, short_version, default_predicates = short_version_kwarg_fixture
-    assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled, short_version=short_version)
+    assert_ok(
+        predicates_lookup, default_predicates, variables=facts,
+        regions_enabled=regions_enabled, short_version=short_version)
 
 
 @pytest.fixture(params=TEST_VARS)
@@ -143,7 +145,9 @@ def short_version_kwarg_fixture(request, facts):
 
 def test_deployment_type_kwarg(predicates_lookup, deployment_type_kwarg_fixture, regions_enabled):
     facts, deployment_type, default_predicates = deployment_type_kwarg_fixture
-    assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled, deployment_type=deployment_type)
+    assert_ok(
+        predicates_lookup, default_predicates, variables=facts,
+        regions_enabled=regions_enabled, deployment_type=deployment_type)
 
 
 @pytest.fixture(params=TEST_VARS)
@@ -153,9 +157,12 @@ def deployment_type_kwarg_fixture(request, facts):
     return facts, deployment_type, default_predicates
 
 
-def test_short_version_deployment_type_kwargs(predicates_lookup, short_version_deployment_type_kwargs_fixture, regions_enabled):
+def test_short_version_deployment_type_kwargs(
+        predicates_lookup, short_version_deployment_type_kwargs_fixture, regions_enabled):
     short_version, deployment_type, default_predicates = short_version_deployment_type_kwargs_fixture
-    assert_ok(predicates_lookup, default_predicates, regions_enabled=regions_enabled, short_version=short_version, deployment_type=deployment_type)
+    assert_ok(
+        predicates_lookup, default_predicates, regions_enabled=regions_enabled,
+        short_version=short_version, deployment_type=deployment_type)
 
 
 @pytest.fixture(params=TEST_VARS)
diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
index 81d3ee19e..1098f9391 100644
--- a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
+++ b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
@@ -119,7 +119,9 @@ def short_version_fixture(request, facts):
 
 def test_short_version_kwarg(priorities_lookup, short_version_kwarg_fixture, zones_enabled):
     facts, short_version, default_priorities = short_version_kwarg_fixture
-    assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled, short_version=short_version)
+    assert_ok(
+        priorities_lookup, default_priorities, variables=facts,
+        zones_enabled=zones_enabled, short_version=short_version)
 
 
 @pytest.fixture(params=TEST_VARS)
@@ -131,7 +133,9 @@ def short_version_kwarg_fixture(request, facts):
 
 def test_deployment_type_kwarg(priorities_lookup, deployment_type_kwarg_fixture, zones_enabled):
     facts, deployment_type, default_priorities = deployment_type_kwarg_fixture
-    assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled, deployment_type=deployment_type)
+    assert_ok(
+        priorities_lookup, default_priorities, variables=facts,
+        zones_enabled=zones_enabled, deployment_type=deployment_type)
 
 
 @pytest.fixture(params=TEST_VARS)
@@ -141,9 +145,12 @@ def deployment_type_kwarg_fixture(request, facts):
     return facts, deployment_type, default_priorities
 
 
-def test_short_version_deployment_type_kwargs(priorities_lookup, short_version_deployment_type_kwargs_fixture, zones_enabled):
+def test_short_version_deployment_type_kwargs(
+        priorities_lookup, short_version_deployment_type_kwargs_fixture, zones_enabled):
     short_version, deployment_type, default_priorities = short_version_deployment_type_kwargs_fixture
-    assert_ok(priorities_lookup, default_priorities, zones_enabled=zones_enabled, short_version=short_version, deployment_type=deployment_type)
+    assert_ok(
+        priorities_lookup, default_priorities, zones_enabled=zones_enabled,
+        short_version=short_version, deployment_type=deployment_type)
 
 
 @pytest.fixture(params=TEST_VARS)
diff --git a/utils/Makefile b/utils/Makefile
index 038c31fcf..e53c0e628 100644
--- a/utils/Makefile
+++ b/utils/Makefile
@@ -23,7 +23,6 @@
 
 NAME := oo-install
 VENV := $(NAME)env
-TESTPACKAGE := oo-install
 SHORTNAME := ooinstall
 
 # This doesn't evaluate until it's called. The -D argument is the
@@ -33,10 +32,6 @@ MANPAGES := docs/man/man1/atomic-openshift-installer.1
 # slipped into the manpage template before a2x processing
 VERSION := 1.4
 
-# YAMLFILES: Skipping all '/files/' folders due to conflicting yaml file definitions
-YAMLFILES = $(shell find ../ -name $(VENV) -prune -o -name .tox -prune -o \( -name '*.yml' -o -name '*.yaml' \) ! -path "*/files/*" -print 2>&1)
-PYFILES = $(shell find ../ -name $(VENV) -prune -o -name ooinstall.egg-info -prune -o -name test -prune -o -name .tox -prune -o -name "*.py" -print)
-
 sdist: clean
 	python setup.py sdist
 	rm -fR $(SHORTNAME).egg-info
diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py
index 0cb37eaff..673997c42 100644
--- a/utils/test/cli_installer_tests.py
+++ b/utils/test/cli_installer_tests.py
@@ -409,8 +409,7 @@ class UnattendedCliTests(OOCliFixture):
         result = self.runner.invoke(cli.cli, self.cli_args)
 
         if result.exception is None or result.exit_code != 1:
-            print("Exit code: %s" % result.exit_code)
-            self.fail("Unexpected CLI return")
+            self.fail("Unexpected CLI return. Exit code: %s" % result.exit_code)
 
     # unattended with config file and all installed hosts (with --force)
     @patch('ooinstall.openshift_ansible.run_main_playbook')
@@ -600,97 +599,6 @@ class UnattendedCliTests(OOCliFixture):
         self.assertEquals('openshift-enterprise',
             inventory.get('OSEv3:vars', 'deployment_type'))
 
-    # 2016-09-26 - tbielawa - COMMENTING OUT these tests FOR NOW while
-    # we wait to see if anyone notices that we took away their ability
-    # to set the ansible_config parameter in the command line options
-    # and in the installer config file.
-    #
-    # We have removed the ability to set the ansible config file
-    # manually so that our new quieter output mode is the default and
-    # only output mode.
-    #
-    # RE: https://trello.com/c/DSwwizwP - atomic-openshift-install
-    # should only output relevant information.
-
-    # @patch('ooinstall.openshift_ansible.run_ansible')
-    # @patch('ooinstall.openshift_ansible.load_system_facts')
-    # def test_no_ansible_config_specified(self, load_facts_mock, run_ansible_mock):
-    #     load_facts_mock.return_value = (MOCK_FACTS, 0)
-    #     run_ansible_mock.return_value = 0
-
-    #     config = SAMPLE_CONFIG % 'openshift-enterprise'
-
-    #     self._ansible_config_test(load_facts_mock, run_ansible_mock,
-    #         config, None, None)
-
-    # @patch('ooinstall.openshift_ansible.run_ansible')
-    # @patch('ooinstall.openshift_ansible.load_system_facts')
-    # def test_ansible_config_specified_cli(self, load_facts_mock, run_ansible_mock):
-    #     load_facts_mock.return_value = (MOCK_FACTS, 0)
-    #     run_ansible_mock.return_value = 0
-
-    #     config = SAMPLE_CONFIG % 'openshift-enterprise'
-    #     ansible_config = os.path.join(self.work_dir, 'ansible.cfg')
-
-    #     self._ansible_config_test(load_facts_mock, run_ansible_mock,
-    #         config, ansible_config, ansible_config)
-
-    # @patch('ooinstall.openshift_ansible.run_ansible')
-    # @patch('ooinstall.openshift_ansible.load_system_facts')
-    # def test_ansible_config_specified_in_installer_config(self,
-    #     load_facts_mock, run_ansible_mock):
-
-    #     load_facts_mock.return_value = (MOCK_FACTS, 0)
-    #     run_ansible_mock.return_value = 0
-
-    #     ansible_config = os.path.join(self.work_dir, 'ansible.cfg')
-    #     config = SAMPLE_CONFIG % 'openshift-enterprise'
-    #     config = "%s\nansible_config: %s" % (config, ansible_config)
-    #     self._ansible_config_test(load_facts_mock, run_ansible_mock,
-    #         config, None, ansible_config)
-
-    # #pylint: disable=too-many-arguments
-    # # This method allows for drastically simpler tests to write, and the args
-    # # are all useful.
-    # def _ansible_config_test(self, load_facts_mock, run_ansible_mock,
-    #     installer_config, ansible_config_cli=None, expected_result=None):
-    #     """
-    #     Utility method for testing the ways you can specify the ansible config.
-    #     """
-
-    #     load_facts_mock.return_value = (MOCK_FACTS, 0)
-    #     run_ansible_mock.return_value = 0
-
-    #     config_file = self.write_config(os.path.join(self.work_dir,
-    #         'ooinstall.conf'), installer_config)
-
-    #     self.cli_args.extend(["-c", config_file])
-    #     if ansible_config_cli:
-    #         self.cli_args.extend(["--ansible-config", ansible_config_cli])
-    #     self.cli_args.append("install")
-    #     result = self.runner.invoke(cli.cli, self.cli_args)
-    #     self.assert_result(result, 0)
-
-    #     # Test the env vars for facts playbook:
-    #     facts_env_vars = load_facts_mock.call_args[0][2]
-    #     if expected_result:
-    #         self.assertEquals(expected_result, facts_env_vars['ANSIBLE_CONFIG'])
-    #     else:
-    #         # If user running test has rpm installed, this might be set to default:
-    #         self.assertTrue('ANSIBLE_CONFIG' not in facts_env_vars or
-    #             facts_env_vars['ANSIBLE_CONFIG'] == cli.DEFAULT_ANSIBLE_CONFIG)
-
-    #     # Test the env vars for main playbook:
-    #     env_vars = run_ansible_mock.call_args[0][2]
-    #     if expected_result:
-    #         self.assertEquals(expected_result, env_vars['ANSIBLE_CONFIG'])
-    #     else:
-    #         # If user running test has rpm installed, this might be set to default:
-    #         #
-    #         # By default we will use the quiet config
-    #         self.assertTrue('ANSIBLE_CONFIG' not in env_vars or
-    #             env_vars['ANSIBLE_CONFIG'] == cli.QUIET_ANSIBLE_CONFIG)
-
     # unattended with bad config file and no installed hosts (without --force)
     @patch('ooinstall.openshift_ansible.run_main_playbook')
     @patch('ooinstall.openshift_ansible.load_system_facts')
@@ -1011,13 +919,7 @@ class AttendedCliTests(OOCliFixture):
             full_line = "%s=%s" % (a, b)
             tokens = full_line.split()
             if tokens[0] == host:
-                found = False
-                for token in tokens:
-                    if token == variable:
-                        found = True
-                        continue
-                self.assertTrue("Unable to find %s in line: %s" %
-                                (variable, full_line), found)
+                self.assertTrue(variable in tokens[1:], "Unable to find %s in line: %s" % (variable, full_line))
                 return
         self.fail("unable to find host %s in inventory" % host)
 
diff --git a/utils/test/fixture.py b/utils/test/fixture.py
index 5200d275d..873ac4a27 100644
--- a/utils/test/fixture.py
+++ b/utils/test/fixture.py
@@ -65,14 +65,13 @@ class OOCliFixture(OOInstallFixture):
 
     def assert_result(self, result, exit_code):
         if result.exit_code != exit_code:
-            print("Unexpected result from CLI execution")
-            print("Exit code: %s" % result.exit_code)
-            print("Exception: %s" % result.exception)
-            print(result.exc_info)
+            msg = ["Unexpected result from CLI execution\n"]
+            msg.append("Exit code: %s\n" % result.exit_code)
+            msg.append("Exception: %s\n" % result.exception)
             import traceback
-            traceback.print_exception(*result.exc_info)
-            print("Output:\n%s" % result.output)
-            self.fail("Exception during CLI execution")
+            msg.extend(traceback.format_exception(*result.exc_info))
+            msg.append("Output:\n%s" % result.output)
+            self.fail("".join(msg))
 
     def _verify_load_facts(self, load_facts_mock):
         """ Check that we ran load facts with expected inputs. """
diff --git a/utils/test/openshift_ansible_tests.py b/utils/test/openshift_ansible_tests.py
index 5847fe37b..02a9754db 100644
--- a/utils/test/openshift_ansible_tests.py
+++ b/utils/test/openshift_ansible_tests.py
@@ -2,7 +2,6 @@ import os
 import unittest
 import tempfile
 import shutil
-import yaml
 
 from six.moves import configparser
 
@@ -40,17 +39,10 @@ class TestOpenShiftAnsible(unittest.TestCase):
     def tearDown(self):
         shutil.rmtree(self.work_dir)
 
-    def generate_hosts(self, num_hosts, name_prefix, roles=None, new_host=False):
-        hosts = []
-        for num in range(1, num_hosts + 1):
-            hosts.append(Host(connect_to=name_prefix + str(num),
-                              roles=roles, new_host=new_host))
-        return hosts
-
     def test_generate_inventory_new_nodes(self):
-        hosts = self.generate_hosts(1, 'master', roles=(['master', 'etcd']))
-        hosts.extend(self.generate_hosts(1, 'node', roles=['node']))
-        hosts.extend(self.generate_hosts(1, 'new_node', roles=['node'], new_host=True))
+        hosts = generate_hosts(1, 'master', roles=(['master', 'etcd']))
+        hosts.extend(generate_hosts(1, 'node', roles=['node']))
+        hosts.extend(generate_hosts(1, 'new_node', roles=['node'], new_host=True))
         openshift_ansible.generate_inventory(hosts)
         inventory = configparser.ConfigParser(allow_no_value=True)
         inventory.read(self.inventory)
@@ -59,8 +51,8 @@ class TestOpenShiftAnsible(unittest.TestCase):
 
     def test_write_inventory_vars_role_vars(self):
         with open(self.inventory, 'w') as inv:
-            openshift_ansible.CFG.deployment.roles['master'].variables={'color': 'blue'}
-            openshift_ansible.CFG.deployment.roles['node'].variables={'color': 'green'}
+            openshift_ansible.CFG.deployment.roles['master'].variables = {'color': 'blue'}
+            openshift_ansible.CFG.deployment.roles['node'].variables = {'color': 'green'}
             openshift_ansible.write_inventory_vars(inv, None)
 
         inventory = configparser.ConfigParser(allow_no_value=True)
@@ -69,3 +61,11 @@ class TestOpenShiftAnsible(unittest.TestCase):
         self.assertEquals('blue', inventory.get('masters:vars', 'color'))
         self.assertTrue(inventory.has_section('nodes:vars'))
         self.assertEquals('green', inventory.get('nodes:vars', 'color'))
+
+
+def generate_hosts(num_hosts, name_prefix, roles=None, new_host=False):
+    hosts = []
+    for num in range(1, num_hosts + 1):
+        hosts.append(Host(connect_to=name_prefix + str(num),
+                          roles=roles, new_host=new_host))
+    return hosts
diff --git a/utils/test/test_utils.py b/utils/test/test_utils.py
index cbce64f7e..cabeaee34 100644
--- a/utils/test/test_utils.py
+++ b/utils/test/test_utils.py
@@ -2,14 +2,14 @@
 Unittests for ooinstall utils.
 """
 
-import six
 import unittest
-import logging
-import sys
 import copy
-from ooinstall.utils import debug_env, is_valid_hostname
 import mock
 
+import six
+
+from ooinstall.utils import debug_env, is_valid_hostname
+
 
 class TestUtils(unittest.TestCase):
     """