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.

unit_tests/charm_tests/test_utils.py

@@ -186,18 +186,18 @@ class TestBaseCharmTest(ut_utils.BaseTestCase):
         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()
 

zaza/openstack/charm_tests/hacluster/tests.py

@@ -86,12 +86,9 @@ class HaclusterScaleBackAndForthTest(HaclusterBaseTest):
         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 @@ class HaclusterScaleBackAndForthTest(HaclusterBaseTest):
         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)

zaza/openstack/charm_tests/test_utils.py

@@ -71,19 +71,20 @@ def skipUntilVersion(service, package, release):
     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 skipVersion(application, package, releases, op, reason):
     :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,