From 8507622ed4b8620ca1fc2a63313bfad3863a29ff Mon Sep 17 00:00:00 2001 From: Corey Bryant Date: Tue, 18 Apr 2023 21:12:15 -0400 Subject: [PATCH] (s/antelope) Change skipVersion decorator to a function The skipVersion decorator was proving to be tricky to use because it took an application name which we didn't want to hard-code, since it is in hacluster/tests.py. One alternative was to pass a charm name instead of application name, but if there were mutiple hacluster applications in the model (e.g. keystone-hacluster and nova-hacluster) we would have to pick just one to test the package version on. After further discussion we decided to change the decorator to a function so that we could use existing class variables to generate the desired hacluster application name. cherry-pick 95fcad2a0a5c9b4960e8ab5e920729cd49c44aa3 From PR#1037 --- unit_tests/charm_tests/test_utils.py | 16 +++---- zaza/openstack/charm_tests/hacluster/tests.py | 17 +++++-- zaza/openstack/charm_tests/test_utils.py | 47 ++++++++----------- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/unit_tests/charm_tests/test_utils.py b/unit_tests/charm_tests/test_utils.py index d2564a202..75257a018 100644 --- a/unit_tests/charm_tests/test_utils.py +++ b/unit_tests/charm_tests/test_utils.py @@ -186,18 +186,18 @@ def test_separate_special_config_None_params(self): self.config_current.assert_called_once_with(None, None) @mock.patch('zaza.openstack.utilities.generic.get_pkg_version') - def test_skipVersion(self, get_pkg_version): - releases = ['4.3.0', '4.0.0'] + def test_package_version_matches(self, get_pkg_version): + versions = ['4.3.0', '4.0.0'] - @test_utils.skipVersion('hacluster', 'crmsh', - releases=releases, - op='eq', - reason='should not run') def _check_should_not_run(): + package_version = test_utils.package_version_matches( + 'hacluster', 'crmsh', versions=versions, op='eq') + if package_version and package_version != '4.4.1': + return raise Exception('should not run') - for release in releases: - get_pkg_version.return_value = release + for version in versions: + get_pkg_version.return_value = version _check_should_not_run() get_pkg_version.reset_mock() diff --git a/zaza/openstack/charm_tests/hacluster/tests.py b/zaza/openstack/charm_tests/hacluster/tests.py index 62ba04291..0aa42affd 100644 --- a/zaza/openstack/charm_tests/hacluster/tests.py +++ b/zaza/openstack/charm_tests/hacluster/tests.py @@ -86,12 +86,9 @@ def setUpClass(cls): test_config = cls.test_config['tests_options']['hacluster'] cls._principle_app_name = test_config['principle-app-name'] cls._hacluster_charm_name = test_config['hacluster-charm-name'] + cls._hacluster_app_name = ("{}-{}".format( + cls._principle_app_name, cls._hacluster_charm_name)) - @test_utils.skipVersion(application='hacluster', - package='crmsh', - releases=['4.4.0-1ubuntu1'], - op='eq', - reason='http://pad.lv/1972730') def test_930_scaleback(self): """Remove one unit, recalculate quorum and re-add one unit. @@ -101,6 +98,16 @@ def test_930_scaleback(self): considers having 3 nodes online out of 4, instead of just 3 out of 3. This test covers this scenario. """ + package = 'crmsh' + package_version = test_utils.package_version_matches( + self._hacluster_app_name, package=package, + versions=['4.4.0-1ubuntu1'], op='eq') + if package_version: + logging.warning("Skipping test on application %s (%s:%s), " + "reason: http://pad.lv/1972730", + self._hacluster_app_name, package, + package_version) + return principle_units = sorted(zaza.model.get_status().applications[ self._principle_app_name]['units'].keys()) self.assertEqual(len(principle_units), 3) diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py index 37dc8ff7e..9f4a8aa92 100644 --- a/zaza/openstack/charm_tests/test_utils.py +++ b/zaza/openstack/charm_tests/test_utils.py @@ -71,19 +71,20 @@ def _skipUntilVersion_inner_2(*args, **kwargs): return _skipUntilVersion_inner_1 -def skipVersion(application, package, releases, op, reason): - """Skip the test if the application is running a versions that matches. +def package_version_matches(application, package, versions, op): + """Determine if the application is running any matching package versions. The version comparison is delegated to `dpkg --compare-versions`, if the - command returns 0, means the release matches, then the test is skipped. + command returns 0, means the version matches. Usage examples: - * Skip the test if hacluster units have crmsh-4.4.0-1ubuntu1 installed + * Return true if hacluster application has crmsh-4.4.0-1ubuntu1 installed - @skipVersion('hacluster', 'crmsh', ['4.4.0-1ubuntu1'], 'eq', - 'LP:# 1234') def test_hacluster(): + if package_version_matches('keystone-hacluster', 'crmsh', + ['4.4.0-1ubuntu1'], 'eq') + return ... :param application: the name of the application to check the package's @@ -92,30 +93,20 @@ def test_hacluster(): :param versions: list of versions to compare with :param op: operation to do the comparison (e.g. lt le eq ne ge gt, see for more details dpkg(1)) - :param reason: The reason logged to skip the test + :return: Matching package version + :rtype: str """ - def _skipVersion_inner_1(f): - def _skipVersion_inner_2(*args, **kwargs): - package_version = generic_utils.get_pkg_version(application, - package) - matches = [] - for release in releases: - p = subprocess.run(['dpkg', '--compare-versions', - package_version, op, release], - stderr=subprocess.STDOUT, - universal_newlines=True) - # match succeeded, the test should be skipped. - matches.append(p.returncode == 0) - if any(matches): - logging.warning("Skipping test on (%s)" - "application %s, reason: %s", - package_version, application, reason) - else: - return f(*args, **kwargs) - - return _skipVersion_inner_2 - return _skipVersion_inner_1 + package_version = generic_utils.get_pkg_version(application, package) + for version in versions: + p = subprocess.run(['dpkg', '--compare-versions', + package_version, op, version], + stderr=subprocess.STDOUT, + universal_newlines=True) + if p.returncode == 0: + logging.info("Package version {version} matches") + return package_version + return None def audit_assertions(action,