From 6bc1b8649cbf549a98d1af90eb6de590b0eb3695 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Thu, 13 Jul 2017 19:31:05 +0200 Subject: Split positive and negative unit tests Split positive and negative tests into their own functions. This means less lines of code, clearer purpose, easier to understand what each test does or doesn't and to add new test cases. --- .../test/aos_version_test.py | 90 ++++++++++------------ 1 file changed, 40 insertions(+), 50 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/test/aos_version_test.py b/roles/openshift_health_checker/test/aos_version_test.py index 697805dd2..532a3e511 100644 --- a/roles/openshift_health_checker/test/aos_version_test.py +++ b/roles/openshift_health_checker/test/aos_version_test.py @@ -18,7 +18,17 @@ expected_pkgs = { } -@pytest.mark.parametrize('pkgs, expect_not_found', [ +@pytest.mark.parametrize('pkgs', [ + # all found + [Package('spam', '3.2.1'), Package('eggs', '3.2.1')], + # found with more specific version + [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')], +]) +def test_check_precise_version_found(pkgs): + aos_version._check_precise_version_found(pkgs, expected_pkgs) + + +@pytest.mark.parametrize('pkgs,expect_not_found', [ ( [], { @@ -54,14 +64,6 @@ expected_pkgs = { } }, # not the right version ), - ( - [Package('spam', '3.2.1'), Package('eggs', '3.2.1')], - {}, # all found - ), - ( - [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')], - {}, # found with more specific version - ), ( [Package('eggs', '1.2.3'), Package('eggs', '3.2.1.5')], { @@ -73,25 +75,22 @@ expected_pkgs = { }, # eggs found with multiple versions ), ]) -def test_check_pkgs_for_precise_version(pkgs, expect_not_found): - if expect_not_found: - with pytest.raises(aos_version.PreciseVersionNotFound) as e: - aos_version._check_precise_version_found(pkgs, expected_pkgs) - - assert list(expect_not_found.values()) == e.value.problem_pkgs - else: +def test_check_precise_version_found_fail(pkgs, expect_not_found): + with pytest.raises(aos_version.PreciseVersionNotFound) as e: aos_version._check_precise_version_found(pkgs, expected_pkgs) + assert list(expect_not_found.values()) == e.value.problem_pkgs -@pytest.mark.parametrize('pkgs, expect_higher', [ - ( - [], - [], - ), - ( - [Package('spam', '3.2.1.9')], - [], # more precise but not strictly higher - ), +@pytest.mark.parametrize('pkgs', [ + [], + # more precise but not strictly higher + [Package('spam', '3.2.1.9')], +]) +def test_check_higher_version_found(pkgs): + aos_version._check_higher_version_found(pkgs, expected_pkgs) + + +@pytest.mark.parametrize('pkgs,expect_higher', [ ( [Package('spam', '3.3')], ['spam-3.3'], # lower precision, but higher @@ -109,28 +108,22 @@ def test_check_pkgs_for_precise_version(pkgs, expect_not_found): ['eggs-3.4'], # multiple versions, two are higher ), ]) -def test_check_pkgs_for_greater_version(pkgs, expect_higher): - if expect_higher: - with pytest.raises(aos_version.FoundHigherVersion) as e: - aos_version._check_higher_version_found(pkgs, expected_pkgs) - assert set(expect_higher) == set(e.value.problem_pkgs) - else: +def test_check_higher_version_found_fail(pkgs, expect_higher): + with pytest.raises(aos_version.FoundHigherVersion) as e: aos_version._check_higher_version_found(pkgs, expected_pkgs) + assert set(expect_higher) == set(e.value.problem_pkgs) -@pytest.mark.parametrize('pkgs, expect_to_flag_pkgs', [ - ( - [], - [], - ), - ( - [Package('spam', '3.2.1')], - [], - ), - ( - [Package('spam', '3.2.1'), Package('eggs', '3.2.2')], - [], - ), +@pytest.mark.parametrize('pkgs', [ + [], + [Package('spam', '3.2.1')], + [Package('spam', '3.2.1'), Package('eggs', '3.2.2')], +]) +def test_check_multi_minor_release(pkgs): + aos_version._check_multi_minor_release(pkgs, expected_pkgs) + + +@pytest.mark.parametrize('pkgs,expect_to_flag_pkgs', [ ( [Package('spam', '3.2.1'), Package('spam', '3.3.2')], ['spam'], @@ -140,10 +133,7 @@ def test_check_pkgs_for_greater_version(pkgs, expect_higher): ['eggs'], ), ]) -def test_check_pkgs_for_multi_release(pkgs, expect_to_flag_pkgs): - if expect_to_flag_pkgs: - with pytest.raises(aos_version.FoundMultiRelease) as e: - aos_version._check_multi_minor_release(pkgs, expected_pkgs) - assert set(expect_to_flag_pkgs) == set(e.value.problem_pkgs) - else: +def test_check_multi_minor_release_fail(pkgs, expect_to_flag_pkgs): + with pytest.raises(aos_version.FoundMultiRelease) as e: aos_version._check_multi_minor_release(pkgs, expected_pkgs) + assert set(expect_to_flag_pkgs) == set(e.value.problem_pkgs) -- cgit v1.2.3 From 163d440065fa1db79f0900a859007c9d8873a40d Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Thu, 13 Jul 2017 19:27:23 +0200 Subject: Make aos_version module handle multiple versions Some packages are supported at more than one major.minor version at the same time. Support is added keeping backward compatibility: the 'version' key can be either a string (single version) or a list of versions. --- .../library/aos_version.py | 28 +++++-- .../test/aos_version_test.py | 89 ++++++++++++++++++---- 2 files changed, 93 insertions(+), 24 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/library/aos_version.py b/roles/openshift_health_checker/library/aos_version.py index 4c205e48c..4f43ee751 100755 --- a/roles/openshift_health_checker/library/aos_version.py +++ b/roles/openshift_health_checker/library/aos_version.py @@ -19,6 +19,10 @@ the inventory, the version comparison checks just pass. ''' from ansible.module_utils.basic import AnsibleModule +# NOTE: because of the dependency on yum (Python 2-only), this module does not +# work under Python 3. But since we run unit tests against both Python 2 and +# Python 3, we use six for cross compatibility in this module alone: +from ansible.module_utils.six import string_types IMPORT_EXCEPTION = None try: @@ -122,12 +126,15 @@ def _check_precise_version_found(pkgs, expected_pkgs_dict): for pkg in pkgs: if pkg.name not in expected_pkgs_dict: continue - # does the version match, to the precision requested? - # and, is it strictly greater, at the precision requested? - expected_pkg_version = expected_pkgs_dict[pkg.name]["version"] - match_version = '.'.join(pkg.version.split('.')[:expected_pkg_version.count('.') + 1]) - if match_version == expected_pkg_version: - pkgs_precise_version_found.add(pkg.name) + expected_pkg_versions = expected_pkgs_dict[pkg.name]["version"] + if isinstance(expected_pkg_versions, string_types): + expected_pkg_versions = [expected_pkg_versions] + for expected_pkg_version in expected_pkg_versions: + # does the version match, to the precision requested? + # and, is it strictly greater, at the precision requested? + match_version = '.'.join(pkg.version.split('.')[:expected_pkg_version.count('.') + 1]) + if match_version == expected_pkg_version: + pkgs_precise_version_found.add(pkg.name) not_found = [] for name, pkg in expected_pkgs_dict.items(): @@ -157,8 +164,13 @@ def _check_higher_version_found(pkgs, expected_pkgs_dict): for pkg in pkgs: if pkg.name not in expected_pkg_names: continue - expected_pkg_version = expected_pkgs_dict[pkg.name]["version"] - req_release_arr = [int(segment) for segment in expected_pkg_version.split(".")] + expected_pkg_versions = expected_pkgs_dict[pkg.name]["version"] + if isinstance(expected_pkg_versions, string_types): + expected_pkg_versions = [expected_pkg_versions] + # NOTE: the list of versions is assumed to be sorted so that the highest + # desirable version is the last. + highest_desirable_version = expected_pkg_versions[-1] + req_release_arr = [int(segment) for segment in highest_desirable_version.split(".")] version = [int(segment) for segment in pkg.version.split(".")] too_high = version[:len(req_release_arr)] > req_release_arr higher_than_seen = version > higher_version_for_pkg.get(pkg.name, []) diff --git a/roles/openshift_health_checker/test/aos_version_test.py b/roles/openshift_health_checker/test/aos_version_test.py index 532a3e511..4100f6c70 100644 --- a/roles/openshift_health_checker/test/aos_version_test.py +++ b/roles/openshift_health_checker/test/aos_version_test.py @@ -18,14 +18,40 @@ expected_pkgs = { } -@pytest.mark.parametrize('pkgs', [ - # all found - [Package('spam', '3.2.1'), Package('eggs', '3.2.1')], - # found with more specific version - [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')], +@pytest.mark.parametrize('pkgs,expected_pkgs_dict', [ + ( + # all found + [Package('spam', '3.2.1'), Package('eggs', '3.2.1')], + expected_pkgs, + ), + ( + # found with more specific version + [Package('spam', '3.2.1'), Package('eggs', '3.2.1.5')], + expected_pkgs, + ), + ( + [Package('ovs', '2.6'), Package('ovs', '2.4')], + { + "ovs": { + "name": "ovs", + "version": ["2.6", "2.7"], + "check_multi": False, + } + }, + ), + ( + [Package('ovs', '2.7')], + { + "ovs": { + "name": "ovs", + "version": ["2.6", "2.7"], + "check_multi": False, + } + }, + ), ]) -def test_check_precise_version_found(pkgs): - aos_version._check_precise_version_found(pkgs, expected_pkgs) +def test_check_precise_version_found(pkgs, expected_pkgs_dict): + aos_version._check_precise_version_found(pkgs, expected_pkgs_dict) @pytest.mark.parametrize('pkgs,expect_not_found', [ @@ -81,36 +107,67 @@ def test_check_precise_version_found_fail(pkgs, expect_not_found): assert list(expect_not_found.values()) == e.value.problem_pkgs -@pytest.mark.parametrize('pkgs', [ - [], - # more precise but not strictly higher - [Package('spam', '3.2.1.9')], +@pytest.mark.parametrize('pkgs,expected_pkgs_dict', [ + ( + [], + expected_pkgs, + ), + ( + # more precise but not strictly higher + [Package('spam', '3.2.1.9')], + expected_pkgs, + ), + ( + [Package('ovs', '2.7')], + { + "ovs": { + "name": "ovs", + "version": ["2.6", "2.7"], + "check_multi": False, + } + }, + ), ]) -def test_check_higher_version_found(pkgs): - aos_version._check_higher_version_found(pkgs, expected_pkgs) +def test_check_higher_version_found(pkgs, expected_pkgs_dict): + aos_version._check_higher_version_found(pkgs, expected_pkgs_dict) -@pytest.mark.parametrize('pkgs,expect_higher', [ +@pytest.mark.parametrize('pkgs,expected_pkgs_dict,expect_higher', [ ( [Package('spam', '3.3')], + expected_pkgs, ['spam-3.3'], # lower precision, but higher ), ( [Package('spam', '3.2.1'), Package('eggs', '3.3.2')], + expected_pkgs, ['eggs-3.3.2'], # one too high ), ( [Package('eggs', '1.2.3'), Package('eggs', '3.2.1.5'), Package('eggs', '3.4')], + expected_pkgs, ['eggs-3.4'], # multiple versions, one is higher ), ( [Package('eggs', '3.2.1'), Package('eggs', '3.4'), Package('eggs', '3.3')], + expected_pkgs, ['eggs-3.4'], # multiple versions, two are higher ), + ( + [Package('ovs', '2.8')], + { + "ovs": { + "name": "ovs", + "version": ["2.6", "2.7"], + "check_multi": False, + } + }, + ['ovs-2.8'], + ), ]) -def test_check_higher_version_found_fail(pkgs, expect_higher): +def test_check_higher_version_found_fail(pkgs, expected_pkgs_dict, expect_higher): with pytest.raises(aos_version.FoundHigherVersion) as e: - aos_version._check_higher_version_found(pkgs, expected_pkgs) + aos_version._check_higher_version_found(pkgs, expected_pkgs_dict) assert set(expect_higher) == set(e.value.problem_pkgs) -- cgit v1.2.3 From a6dbc9ff06b6a245317de6279e7b3c88ff0ef1c3 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Thu, 13 Jul 2017 19:44:41 +0200 Subject: Allow OVS 2.7 in latest OpenShift releases Change the package_version check to tolerate either Open vSwitch 2.6 or 2.7. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1465882 This commit removes a unit test that adds no coverage and tests data instead of logic. This coupling makes every change to supported versions require the same changes to the tests. --- .../openshift_checks/package_version.py | 4 +-- .../test/package_version_test.py | 30 ---------------------- 2 files changed, 2 insertions(+), 32 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index 6a76bb93d..204752bd0 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -10,8 +10,8 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck): tags = ["preflight"] openshift_to_ovs_version = { - "3.6": "2.6", - "3.5": "2.6", + "3.6": ["2.6", "2.7"], + "3.5": ["2.6", "2.7"], "3.4": "2.4", } diff --git a/roles/openshift_health_checker/test/package_version_test.py b/roles/openshift_health_checker/test/package_version_test.py index 91eace512..1bb6371ae 100644 --- a/roles/openshift_health_checker/test/package_version_test.py +++ b/roles/openshift_health_checker/test/package_version_test.py @@ -72,36 +72,6 @@ def test_package_version(openshift_release): assert result is return_value -@pytest.mark.parametrize('deployment_type,openshift_release,expected_ovs_version', [ - ("openshift-enterprise", "3.5", "2.6"), - ("origin", "3.6", "2.6"), - ("openshift-enterprise", "3.4", "2.4"), - ("origin", "3.3", "2.4"), -]) -def test_ovs_package_version(deployment_type, openshift_release, expected_ovs_version): - task_vars = dict( - openshift=dict(common=dict(service_type='origin')), - openshift_release=openshift_release, - openshift_image_tag='v' + openshift_release, - openshift_deployment_type=deployment_type, - ) - return_value = object() - - def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None): - assert module_name == 'aos_version' - assert "package_list" in module_args - - for pkg in module_args["package_list"]: - if pkg["name"] == "openvswitch": - assert pkg["version"] == expected_ovs_version - - return return_value - - check = PackageVersion(execute_module=execute_module) - result = check.run(tmp=None, task_vars=task_vars) - assert result is return_value - - @pytest.mark.parametrize('deployment_type,openshift_release,expected_docker_version', [ ("origin", "3.5", "1.12"), ("openshift-enterprise", "3.4", "1.12"), -- cgit v1.2.3