cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:feature/skip-better into cloud-init:master.

Commit message:
tests: allow skipping an entire cloud_test without running.

Individual skipTest or setUp SkipTest will still launch the instance.
This allows us to stop the running of the instance so we don't
waste cycles or boot systems that are known to fail.


Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/355870

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/skip-better into cloud-init:master.

diff --git a/tests/cloud_tests/collect.py b/tests/cloud_tests/collect.py
index 75b5061..642745d 100644
--- a/tests/cloud_tests/collect.py
+++ b/tests/cloud_tests/collect.py
@@ -9,6 +9,7 @@ from cloudinit import util as c_util
 from tests.cloud_tests import (config, LOG, setup_image, util)
 from tests.cloud_tests.stage import (PlatformComponent, run_stage, run_single)
 from tests.cloud_tests import platforms
+from tests.cloud_tests.testcases import base, get_test_class
 
 
 def collect_script(instance, base_dir, script, script_name):
@@ -63,6 +64,7 @@ def collect_test_data(args, snapshot, os_name, test_name):
     res = ({}, 1)
 
     # load test config
+    test_name_in = test_name
     test_name = config.path_to_name(test_name)
     test_config = config.load_test_config(test_name)
     user_data = test_config['cloud_config']
@@ -75,6 +77,16 @@ def collect_test_data(args, snapshot, os_name, test_name):
         LOG.warning('test config %s is not enabled, skipping', test_name)
         return ({}, 0)
 
+    test_class = get_test_class(
+        config.name_to_module(test_name_in),
+        test_data={'platform': snapshot.platform_name, 'os_name': os_name},
+        test_conf=test_config['cloud_config'])
+    try:
+        test_class.maybeSkipTest()
+    except base.SkipTest as s:
+        LOG.warning('skipping test config %s: %s', test_name, s)
+        return ({}, 0)
+
     # if testcase requires a feature flag that the image does not support,
     # skip the testcase with a warning
     req_features = test_config.get('required_features', [])
diff --git a/tests/cloud_tests/testcases/__init__.py b/tests/cloud_tests/testcases/__init__.py
index bd548f5..d300b12 100644
--- a/tests/cloud_tests/testcases/__init__.py
+++ b/tests/cloud_tests/testcases/__init__.py
@@ -4,7 +4,7 @@
 
 import importlib
 import inspect
-import unittest
+import unittest2
 from unittest.util import strclass
 
 from cloudinit.util import read_conf
@@ -25,35 +25,48 @@ def discover_tests(test_name):
     except NameError:
         raise ValueError('no test verifier found at: {}'.format(testmod_name))
 
-    return [mod for name, mod in inspect.getmembers(testmod)
-            if inspect.isclass(mod) and base_test in inspect.getmro(mod) and
-            getattr(mod, '__test__', True)]
+    found = [mod for name, mod in inspect.getmembers(testmod)
+             if (inspect.isclass(mod)
+                 and base_test in inspect.getmro(mod)
+                 and getattr(mod, '__test__', True))]
+    if len(found) != 1:
+        raise RuntimeError(
+            "Unexpected situation, multiple tests for %s: %s" % (
+                test_name, found))
 
+    return found
 
-def get_suite(test_name, data, conf):
-    """Get test suite with all tests for 'testname'.
 
-    @return_value: a test suite
-    """
-    suite = unittest.TestSuite()
-    for test_class in discover_tests(test_name):
+def get_test_class(test_name, test_data, test_conf):
+    test_class = discover_tests(test_name)[0]
+
+    class DynamicTestSubclass(test_class):
 
-        class tmp(test_class):
+        _realclass = test_class
+        data = test_data
+        conf = test_conf
+        release_conf = read_conf(config.RELEASES_CONF)['releases']
 
-            _realclass = test_class
+        def __str__(self):
+            return "%s (%s)" % (self._testMethodName,
+                                strclass(self._realclass))
 
-            def __str__(self):
-                return "%s (%s)" % (self._testMethodName,
-                                    strclass(self._realclass))
+        @classmethod
+        def setUpClass(cls):
+            cls.maybeSkipTest()
 
-            @classmethod
-            def setUpClass(cls):
-                cls.data = data
-                cls.conf = conf
-                cls.release_conf = read_conf(config.RELEASES_CONF)['releases']
+    return DynamicTestSubclass
 
-        suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(tmp))
 
+def get_suite(test_name, data, conf):
+    """Get test suite with all tests for 'testname'.
+
+    @return_value: a test suite
+    """
+    suite = unittest2.TestSuite()
+    suite.addTest(
+        unittest2.defaultTestLoader.loadTestsFromTestCase(
+            get_test_class(test_name, data, conf)))
     return suite
 
 # vi: ts=4 expandtab
diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py
index e83cb9a..e18d601 100644
--- a/tests/cloud_tests/testcases/base.py
+++ b/tests/cloud_tests/testcases/base.py
@@ -31,6 +31,11 @@ class CloudTestCase(unittest2.TestCase):
     def is_distro(self, distro_name):
         return self.os_cfg['os'] == distro_name
 
+    @classmethod
+    def maybeSkipTest(cls):
+        """Present to allow subclasses to override and raise a skipTest."""
+        pass
+
     def assertPackageInstalled(self, name, version=None):
         """Check dpkg-query --show output for matching package name.
 
diff --git a/tests/cloud_tests/testcases/modules/lxd_bridge.py b/tests/cloud_tests/testcases/modules/lxd_bridge.py
index 8697ae9..ea545e0 100644
--- a/tests/cloud_tests/testcases/modules/lxd_bridge.py
+++ b/tests/cloud_tests/testcases/modules/lxd_bridge.py
@@ -7,14 +7,15 @@ from tests.cloud_tests.testcases import base
 class TestLxdBridge(base.CloudTestCase):
     """Test LXD module."""
 
-    def setUp(self):
+    @classmethod
+    def maybeSkipTest(cls):
         """Skip on cosmic for two reasons:
         a.) LP: #1795036 - 'lxd init' fails on cosmic kernel.
         b.) apt install lxd installs via snap which can be slow
             as that will download core snap and lxd."""
-        if self.os_name == "cosmic":
-            raise self.skipTest('Skipping test on cosmic (LP: #1795036).')
-        return base.CloudTestCase.setUp(self)
+        os_name = cls.data.get('os_name', 'UNKNOWN')
+        if os_name == "cosmic":
+            raise base.SkipTest('Skipping test on cosmic (LP: #1795036).')
 
     def test_lxd(self):
         """Test lxd installed."""
diff --git a/tests/cloud_tests/testcases/modules/lxd_dir.py b/tests/cloud_tests/testcases/modules/lxd_dir.py
index 8bd8c6e..797bafe 100644
--- a/tests/cloud_tests/testcases/modules/lxd_dir.py
+++ b/tests/cloud_tests/testcases/modules/lxd_dir.py
@@ -7,14 +7,15 @@ from tests.cloud_tests.testcases import base
 class TestLxdDir(base.CloudTestCase):
     """Test LXD module."""
 
-    def setUp(self):
+    @classmethod
+    def maybeSkipTest(cls):
         """Skip on cosmic for two reasons:
         a.) LP: #1795036 - 'lxd init' fails on cosmic kernel.
         b.) apt install lxd installs via snap which can be slow
             as that will download core snap and lxd."""
-        if self.os_name == "cosmic":
-            raise self.skipTest('Skipping test on cosmic (LP: #1795036).')
-        return base.CloudTestCase.setUp(self)
+        os_name = cls.data.get('os_name', 'UNKNOWN')
+        if os_name == "cosmic":
+            raise base.SkipTest('Skipping test on cosmic (LP: #1795036).')
 
     def test_lxd(self):
         """Test lxd installed."""