Merge lp:~marcoceppi/charm-tools/substrate-cfg into lp:charm-tools/1.2

[Marco Ceppi]

Add substrate configuration to juju-test

https://codereview.appspot.com/49940044/

[Marco Ceppi (marcoceppi)]

Reviewers: mp+201094_code.launchpad.net,

Message:
Please take a look.

Description:
Updates to bsaller's fixes to substrate parsing

https://code.launchpad.net/~marcoceppi/charm-tools/substrate-cfg/+merge/201094

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/49940044/

Affected files (+189, -37 lines):
   M .bzrignore
   A [revision details]
   M charmtools/bundles.py
   M charmtools/generate.py
   M charmtools/test.py
   M charmtools/update.py
   M tests/test_juju_test.py
   A tests/test_substrates.py

[Benjamin Saller (bcsaller)]

LGTM

Thanks for including my suggestions (and making the actual integration
work). The changes to using a key whitelist in TestCfg are also good.

https://codereview.appspot.com/49940044/

[Marco Ceppi (marcoceppi)]

*** Submitted:

Add substrate configuration to juju-test

R=benjamin.saller
CC=
https://codereview.appspot.com/49940044

https://codereview.appspot.com/49940044/

=== modified file '.bzrignore'
--- .bzrignore	2013-12-16 16:12:29 +0000
+++ .bzrignore	2014-01-09 20:17:53 +0000
@@ -9,3 +9,5 @@
 lib
 local
 man
+tests/.ropeproject/
+charmtools/.ropeproject/
 
=== modified file 'charmtools/bundles.py'
--- charmtools/bundles.py	2013-12-20 14:06:43 +0000
+++ charmtools/bundles.py	2014-01-09 20:17:53 +0000
@@ -2,7 +2,6 @@
 import yaml
 import glob
 import json
-import requests
 
 from linter import Linter
 from charmworldlib import bundle as cw_bundle
 
=== modified file 'charmtools/generate.py'
--- charmtools/generate.py	2013-12-12 17:20:39 +0000
+++ charmtools/generate.py	2014-01-09 20:17:53 +0000
@@ -16,7 +16,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-import sys
 import shutil
 import argparse
 
 
=== modified file 'charmtools/test.py'
--- charmtools/test.py	2013-12-21 12:59:08 +0000
+++ charmtools/test.py	2014-01-09 20:17:53 +0000
@@ -1,18 +1,16 @@
 #!/usr/bin/python
 # coding=latin-1
 
+import argparse
+import glob
+import logging
 import os
+import re
+import signal
+import subprocess
 import sys
-import uuid
+import time
 import yaml
-import time
-import glob
-import signal
-import logging
-import argparse
-import subprocess
-
-from tempfile import mkdtemp
 from datetime import timedelta
 from contextlib import contextmanager
 
@@ -44,6 +42,10 @@
     pass
 
 
+class SubstrateMismatch(Exception):
+    pass
+
+
 class TimeoutError(Exception):
     def __init__(self, value="Timed Out"):
         self.value = value
@@ -114,7 +116,7 @@
                 self.destroy(self.juju_env)
             except DestroyUnreliable:
                 self.log.warn('Unable to destroy bootstrap, trying again')
-                sleep(2)
+                time.sleep(2)
                 try:
                     self.destroy(self.juju_env)
                 except:
@@ -258,6 +260,7 @@
         try:
             with timeout(self.conductor.args.timeout):
                 output = subprocess.check_output(self.test, env=self.env)
+                self.log.debug(output)
         except TimeoutError, e:
             self.log.debug('Killed by timeout after %s seconds',
                            self.conductor.args.timeout)
@@ -279,7 +282,7 @@
             try:
                 self.archive_logs()
             except OrchestraError, e:
-                juju.log.error(e)
+                self.log.error(e)
 
         if error:
             raise error
@@ -293,7 +296,7 @@
             raise OrchestraError('Unable to query juju status')
 
         services = status['services']
-        machines = status['machines']
+        # machines = status['machines']
 
         if self.conductor.juju_version.major == 0:
             logs.append('/var/lib/juju/units/./*/charm.log')
@@ -395,16 +398,16 @@
 class TestCfg(object):
     _keys = ['timeout', 'set-e', 'on-timeout', 'fail-on-skip', 'tests']
 
-    def __init__(self, config_file):
-        if not os.path.exists(config_file):
-            raise OSError("%s not found" % config_file)
-
-        with open(config_file) as f:
-            cfg = yaml.safe_load(f.read())
-
-        for key, val in cfg['options'].iteritems():
-            if key in self._keys:
-                setattr(self, key, val)
+    def __init__(self, cfg):
+        if isinstance(cfg, basestring):
+            cfg = yaml.safe_load(cfg)
+
+        if 'options' in cfg:
+            for key, val in cfg['options'].iteritems():
+                if key in self._keys:
+                    setattr(self, key, val)
+        if 'substrates' in cfg:
+            self.substrates = cfg.substrates
 
 
 def get_juju_version():
@@ -455,6 +458,71 @@
     return logger
 
 
+class SubstrateFilter(object):
+    def __init__(self, spec):
+        self.order = spec.get('order', ['include', 'skip'])
+        self.include = spec.get('include', ['*'])
+        self.skip = spec.get('skip', [])
+
+        if isinstance(self.order, str):
+            self.order = [s.strip() for s in self.order.split(',')]
+        if self.order != ['include', 'skip'] and \
+                self.order != ['skip', 'include']:
+            raise ValueError(
+                'order should be defined using only include and skip')
+
+        if isinstance(self.include, str):
+            self.include = [self.include]
+        self.include = set(self.include)
+
+        if isinstance(self.skip, str):
+            self.skip = [self.skip]
+        self.skip = set(self.skip)
+
+    def filter(self, substrates):
+        """
+        Filter a list of substrates relative to the rules generated on class
+        creation.
+        """
+        if isinstance(substrates, str):
+            substrates = [s.strip() for s in re.split('[,\s]', substrates)]
+
+        # Apply the rules to the list of substrates returning anything that
+        # matches
+        if self.order == ['include', 'skip']:
+            result = self._filter_includes(substrates, True)
+            result = self._filter_skips(result)
+        else:
+            result = self._filter_skips(substrates, True)
+            result = self._filter_includes(result)
+        return result
+
+    def _filter_includes(self, inputList, priority=False):
+        if priority and '*' in self.include:
+            return inputList
+        return sorted(list(set(inputList).intersection(self.include)))
+
+    def _filter_skips(self, inputList, priority=False):
+        if priority and '*' in self.skip:
+            return list(self.include.intersection(inputList))
+        return sorted(list(set(inputList).difference(self.skip)))
+
+
+def parse_substrates(spec):
+    if isinstance(spec, basestring):
+        spec = yaml.safe_load(spec)
+    if not spec or 'substrates' not in spec:
+        raise ValueError(
+            "Invalid data passed to parse_substrates: {}".format(spec))
+
+    specRules = SubstrateFilter(spec['substrates'])
+    return specRules
+
+
+def allowed_substrates(spec, possible_substrates):
+    return parse_substrates(spec).filter(possible_substrates)
+
+
 def setup_parser():
     parser = argparse.ArgumentParser(
         prog='juju test',
@@ -564,10 +632,14 @@
     logger.info('Starting test run on %s using Juju %s'
                 % (args.juju_env, get_juju_version()))
     logger.debug('Loading configuration options from testplan YAML')
-    test_plans = glob.glob(os.path.join(os.getcwd(), 'tests', 'testplan.y*ml'))
-    test_plan = test_plan[0] if test_plans else None
-    if test_plan:
-        cfg = TestCfg(test_plan)
+    test_plans = glob.glob(os.path.join(os.getcwd(), 'tests',
+                                        'test_config.y*ml'))
+    if test_plans:
+        with open(test_plans[0]) as f:
+            test_cfg = f.read()
+
+    if test_cfg:
+        cfg = TestCfg(test_cfg)
         for key, val in args.iteritems():
             logger.debug('Overwriting %s to %s from cmd' % (key, val))
             setattr(cfg, key, val)
@@ -577,10 +649,21 @@
     logger.debug('Creating a new Conductor')
     try:
         tester = Conductor(args)
+        env_yaml = tester.get_environment(cfg.juju_env)
+        if 'substrates' in cfg:
+            rules = parse_substrates(cfg)
+            allowed = rules.filter(env_yaml['type'])
+            if env_yaml['type'] not in allowed:
+                raise Exception('%s is not an allowed substrate: %s' %
+                                (env_yaml['type'],
+                                 allowed.join(', ')))
         errors, failures, passes = tester.run()
     except NoTests:
         logger.critical('No tests were found')
         sys.exit(1)
+    except Exception as e:
+        logger.critical(str(e))
+        sys.exit(1)
     except:
         raise
 
 
=== modified file 'charmtools/update.py'
--- charmtools/update.py	2013-12-16 16:12:29 +0000
+++ charmtools/update.py	2014-01-09 20:17:53 +0000
@@ -34,6 +34,7 @@
 
     return parser
 
+
 def update(charm_dir, fix=False):
     mr = Mr(charm_dir)
     for charm in charms.remote():
@@ -48,6 +49,7 @@
     except Exception as e:
         raise Exception(".mrconfig not saved: %s" % e.strerror)
 
+
 def main():
     parser = setup_parser()
     args = parser.parse_args()
 
=== modified file 'tests/test_juju_test.py'
--- tests/test_juju_test.py	2013-12-20 12:33:21 +0000
+++ tests/test_juju_test.py	2014-01-09 20:17:53 +0000
@@ -30,13 +30,16 @@
 
 PARSED_ENVIRONMENTS_YAML = yaml.safe_load(RAW_ENVIRONMENTS_YAML)
 
+
 class Arguments(object):
     def __init__(self, **kwargs):
         for key, value in kwargs.items():
             setattr(self, key, value)
+
     def __getattr__(self, name):
         return None
 
+
 class JujuTestPluginTest(unittest.TestCase):
     @patch('subprocess.check_output')
     def test_get_gojuju_version(self, mock_check_output):
@@ -77,7 +80,7 @@
         self.assertEqual(100, juju_test.convert_to_timedelta('100s'))
         self.assertEqual(100, juju_test.convert_to_timedelta(100))
         self.assertEqual(60, juju_test.convert_to_timedelta('1m'))
-        self.assertEqual(60*60, juju_test.convert_to_timedelta('1h'))
+        self.assertEqual(60 * 60, juju_test.convert_to_timedelta('1h'))
 
     @patch('glob.glob')
     @patch('os.path.isfile')
@@ -89,7 +92,7 @@
         mock_isfile.side_effect = files_exist
         mock_glob.return_value = tests_directory
 
-        args = Arguments(tests = 'dummy')
+        args = Arguments(tests='dummy')
         c = juju_test.Conductor(args)
         results = c.find_tests()
 
@@ -100,7 +103,7 @@
     def test_conductor_find_tests_fails(self, mock_glob):
         mock_glob.return_value = []
 
-        args = Arguments(tests = 'dummy')
+        args = Arguments(tests='dummy')
         c = juju_test.Conductor(args)
         results = c.find_tests()
 
@@ -126,7 +129,7 @@
 
         mcheck_output.side_effect = [yml_output, Exception('not bootstrapped')]
         juju_env = 'test'
-        args = Arguments(tests = 'dummy')
+        args = Arguments(tests='dummy')
         c = juju_test.Conductor(args)
         results = c.status(juju_env)
 
@@ -193,7 +196,7 @@
 
     @patch('subprocess.check_call')
     def test_conductor_destroy(self, mock_check_call):
-        args = Arguments(tests = 'dummy')
+        args = Arguments(tests='dummy')
         c = juju_test.Conductor(args)
         c.juju_version = juju_test.JujuVersion(major=1, minor=1, patch=1)
         good_env = 'valid'
@@ -216,7 +219,7 @@
 
         bad_env = 'invalid'
 
-        args = Arguments(tests = 'dummy')
+        args = Arguments(tests='dummy')
         c = juju_test.Conductor(args)
         c.juju_version = juju_test.JujuVersion(major=1, minor=8, patch=0)
         self.assertRaises(juju_test.DestroyUnreliable, c.destroy, bad_env)
@@ -448,7 +451,7 @@
         o.log.status.assert_called_with('%s (%s)' % (juju_test.TEST_FAIL,
                                                      juju_test.TEST_TIMEOUT))
 
-        o.conductor.args.on_timeout='skip'
+        o.conductor.args.on_timeout = 'skip'
         o.log.status.reset_mock()
         o.print_status(124)
         o.log.status.assert_called_with('%s (%s)' % (juju_test.TEST_FAIL,
@@ -583,7 +586,7 @@
         goyml_parsed = yaml.safe_load(goyml_output)
         mstatus.return_value = goyml_parsed
 
-        juju_env='testing'
+        juju_env = 'testing'
         args = Arguments(tests='dummy', juju_env=juju_env, logdir='/tmp')
         c = juju_test.Conductor(args)
         c.juju_version = juju_test.JujuVersion(major=1, minor=10, patch=0)
@@ -623,7 +626,7 @@
         pyyml_parsed = yaml.safe_load(pyyml_output)
         mstatus.return_value = pyyml_parsed
 
-        juju_env='testing'
+        juju_env = 'testing'
         args = Arguments(tests='dummy', juju_env=juju_env, logdir='/tmp')
         c = juju_test.Conductor(args)
         c.juju_version = juju_test.JujuVersion(major=0, minor=7, patch=0)
 
=== added file 'tests/test_substrates.py'
--- tests/test_substrates.py	1970-01-01 00:00:00 +0000
+++ tests/test_substrates.py	2014-01-09 20:17:53 +0000
@@ -0,0 +1,62 @@
+"""Unit test for juju tests substrate handling"""
+
+import unittest
+import yaml
+from charmtools.test import parse_substrates, allowed_substrates
+
+
+class JujuTestSubstrateTests(unittest.TestCase):
+    INCLUDE_SKIP = '''
+substrates:
+    order: include, skip
+    skip: amazon
+    include: "*"
+    '''
+
+    SKIP_INCLUDE = '''
+substrates:
+    order: skip, include
+    skip: "*"
+    include: amazon
+    '''
+
+    def test_parse_substrates_text(self):
+        self.assertRaises(ValueError,  parse_substrates, '')
+        self.assertIsNotNone(parse_substrates(self.INCLUDE_SKIP))
+
+    def test_parse_substrate_object(self):
+        data = yaml.load(self.INCLUDE_SKIP)
+        self.assertIsNotNone(parse_substrates(data))
+
+    def test_parse_substrates_order(self):
+        result = parse_substrates(self.INCLUDE_SKIP)
+        self.assertEqual(result.order, ['include', 'skip'])
+
+    def test_parse_substrates_order_invalid(self):
+        data = self.INCLUDE_SKIP.replace('skip', 'foobar')
+        self.assertRaises(ValueError, parse_substrates, data)
+
+    def test_parse_substrates_include(self):
+        result = parse_substrates(self.INCLUDE_SKIP)
+        self.assertEqual(result.include, set(['*']))
+
+    def test_parse_substrates_skip(self):
+        result = parse_substrates(self.INCLUDE_SKIP)
+        self.assertEqual(result.skip, set(['amazon']))
+
+    def test_filter_include_skip(self):
+        rules = parse_substrates(self.INCLUDE_SKIP)
+        self.assertEqual(rules.filter('amazon,hp,azure'), ['azure', 'hp'])
+
+    def test_filter_skip_include(self):
+        rules = parse_substrates(self.SKIP_INCLUDE)
+        self.assertEqual(rules.filter('amazon,hp,azure'), ['amazon'])
+
+    def test_allowed_substrates(self):
+        self.assertEqual(allowed_substrates(self.INCLUDE_SKIP,
+                                            ['foo', 'bar', 'amazon']),
+                         ['bar', 'foo'])
+
+
+if __name__ == '__main__':
+    unittest.main()