From e938abbf8be7e56aaba1638d0f36b2974fbef302 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 27 Feb 2017 16:21:42 +0100 Subject: Remove unused Makefile variables --- utils/Makefile | 5 ----- 1 file changed, 5 deletions(-) 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 -- cgit v1.2.1 From e174c88d864066d4efb5b46d4767bc167341c421 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 27 Feb 2017 16:58:18 +0100 Subject: Remove unused argument Detected by pylint. The fixture indeed doesn't require an argument. --- roles/openshift_master_facts/test/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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': {} -- cgit v1.2.1 From ca3cf657bf4b6676166579065926bf315a73e416 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 27 Feb 2017 17:28:12 +0100 Subject: Rewrap long lines --- .../openshift_master_facts_default_predicates_tests.py | 15 +++++++++++---- .../openshift_master_facts_default_priorities_tests.py | 15 +++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) 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) -- cgit v1.2.1 From 2e0aa00fe7d4a82c0267191deee1b7613787ab9d Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 27 Feb 2017 22:11:13 +0100 Subject: Lint utils/test - Do not use `print` in unit tests, send messages through the test framework instead. - Remove unused import. - Add spaces around equal sign in assigment. - Turn method into a function. - Reorganize imports according to PEP8. --- utils/test/cli_installer_tests.py | 3 +-- utils/test/fixture.py | 13 ++++++------- utils/test/openshift_ansible_tests.py | 26 +++++++++++++------------- utils/test/test_utils.py | 8 ++++---- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 0cb37eaff..067bb9de7 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') 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): """ -- cgit v1.2.1 From 73b1a388623cf3336e97502280c1a97852b31411 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 28 Feb 2017 17:21:36 +0100 Subject: Fix test Instead of checking if a string is True, check if 'found' is True, the string is the error message. Also, we can remove the loop and use the simpler Python 'in' construct. --- utils/test/cli_installer_tests.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 067bb9de7..21e0ec722 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -1010,13 +1010,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) -- cgit v1.2.1 From 3577754a29953c00f65f9b0b7ad257597c9ceb2d Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 6 Mar 2017 10:35:15 +0100 Subject: Remove redundant assertion That line is testing Python's list.count method, instead of yedit. The assertion right above is a superset of it, as it checks for equality to some expected value. --- roles/lib_utils/src/test/unit/test_yedit.py | 2 -- 1 file changed, 2 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): -- cgit v1.2.1 From 3763417a1351448d2be60afb227af138aa22818e Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 28 Feb 2017 17:16:10 +0100 Subject: Remove old commented-out tests --- utils/test/cli_installer_tests.py | 91 --------------------------------------- 1 file changed, 91 deletions(-) diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py index 21e0ec722..673997c42 100644 --- a/utils/test/cli_installer_tests.py +++ b/utils/test/cli_installer_tests.py @@ -599,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') -- cgit v1.2.1