Merge lp:~marcoceppi/charm-tools/fix-add-tests into lp:charm-tools/1.4

[Marco Ceppi]

[Tim Van Steenburgh (tvansteenburgh)]

This looks good, I just have a few comments, using this snip from the test I created as an example:

```
#!/usr/bin/env python3

import amulet
import requests
import unittest

class TestDeployment(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        cls.deployment = amulet.Deployment(series='trusty')

        cls.deployment.add('thinkup')
        cls.deployment.add('cs:precise/apache2-25')
        cls.deployment.add('cs:precise/mysql-50')

        cls.deployment.relate('thinkup:website', 'apache2')
        cls.deployment.relate('thinkup:db', 'mysql')
```

1. We should install python3-requests in 00-setup since it's imported by 99-autogen.

2. cls.deployment.add('cs:precise/apache2-25') - I think we should just add 'apache2' here, and let amulet get the latest revision in the correct series.

3. cls.deployment.relate('thinkup:website', 'apache2') - Amulet will choke on this b/c the 2nd arg does not explicitly name the relation. Can this be corrected?

4. I agree with you about the templating approach, it's not ideal. Better would be to make body.tpl a Jinja template and do the looping directly in the template. Not a big deal though, we could clean that up later if you don't feel like doing it now.

review: Needs Fixing

[lp:~marcoceppi/charm-tools/fix-add-tests]

345. By Marco Ceppi on 2014-11-05

Feedback from Tim

346. By Marco Ceppi on 2014-11-05

python3

[Tim Van Steenburgh (tvansteenburgh)]

LGTM.

review: Approve

=== modified file 'charmtools/generate.py'
--- charmtools/generate.py	2014-07-01 15:35:22 +0000
+++ charmtools/generate.py	2014-11-05 18:11:03 +0000
@@ -51,18 +51,18 @@
     shutil.copy(os.path.join(CHARM_TPL, tpl_file), charm_dir)
 
 
-def tests(charm_dir, is_bundle=False, debug=False):
+def tests(charm_dir, is_bundle=False, debug=False, series='trusty'):
     c = Charm(charm_dir)
 
+    if not c.is_charm():
+        raise Exception('Not a Charm')
+
+    mdata = c.metadata()
+
     interfaces = {}
-    deploy = []
+    deploy = [mdata['name']]
     relations = []
 
-    if not c.is_charm():
-        raise Exception('Not a Charm')
-
-    mdata = c.metadata()
-
     for rel_type in ['provides', 'requires']:
         if rel_type in mdata:
             interfaces[rel_type] = {}
@@ -81,12 +81,13 @@
     d = Template(file=ATPL['deploy'], searchList=[{'services': deploy}])
     s = Template(file=ATPL['relate'], searchList=[{'relations': relations}])
 
-    t = Template(file=ATPL['body'], searchList=[{'deploy': d, 'relate': s}])
+    t = Template(file=ATPL['body'], searchList=[{'deploy': d, 'relate': s,
+                                                 'series': series}])
 
     if not os.path.exists(os.path.join(charm_dir, 'tests')):
         os.mkdir(os.path.join(charm_dir, 'tests'))
 
-    with open(os.path.join(charm_dir, 'tests', '00-autogen'), 'w') as f:
+    with open(os.path.join(charm_dir, 'tests', '99-autogen'), 'w') as f:
         f.write(str(t))
 
     if not os.path.exists(os.path.join(charm_dir, 'tests', '00-setup')):
@@ -98,7 +99,7 @@
 sudo apt-get install amulet -y
 """)
 
-    os.chmod(os.path.join(charm_dir, 'tests', '00-autogen'), 0755)
+    os.chmod(os.path.join(charm_dir, 'tests', '99-autogen'), 0755)
     os.chmod(os.path.join(charm_dir, 'tests', '00-setup'), 0755)
 
 
@@ -108,13 +109,23 @@
     parser.add_argument('subcommand', choices=['tests', 'readme', 'icon'],
                         help='Which type of generator to run')
     parser = parser_defaults(parser)
+    return parser.parse_known_args(args)
+
+
+def tests_parser(args):
+    # This bites, need an argparser experter
+    parser = argparse.ArgumentParser(description="Add tests to a charm")
+    parser.add_argument('--series', '-s', default='trusty',
+                        help='Series for the generated test')
     return parser.parse_args(args)
 
 
 def main():
-    a = parser()
+    a, extra = parser()
     if a.subcommand == 'tests':
-        tests(os.getcwd(), is_bundle=a.bundle, debug=a.debug)
+        opts = tests_parser(extra)
+        tests(os.getcwd(), is_bundle=a.bundle, debug=a.debug,
+              series=opts.series)
     elif a.subcommand == 'readme':
         copy_file('README.ex', os.getcwd(), is_bundle=a.bundle, debug=a.debug)
     elif a.subcommand == 'icon':

=== modified file 'charmtools/templates/tests/body.tpl'
--- charmtools/templates/tests/body.tpl	2013-12-10 17:37:20 +0000
+++ charmtools/templates/tests/body.tpl	2014-11-05 18:11:03 +0000
@@ -1,30 +1,56 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 
 import amulet
-
-d = amulet.Deployment()
+import requests
+import unittest
+
+
+class TestDeployment(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.deployment = amulet.Deployment(series='$series')
 
 $deploy
 $relate
-# Don't forget to expose using d.expose(service)
-
-try:
-    d.setup(timeout=900)
-    d.sentry.wait()
-except amulet.helpers.TimeoutError:
-    amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
-except:
-    raise
-
-# Now you can use d.sentry.unit[UNIT] to address each of the units and perform
-# more in-depth steps. There are three test statuses: amulet.PASS, amulet.FAIL,
-# and amulet.SKIP - these can be triggered with amulet.raise_status(). Each
-# d.sentry.unit[] has the following methods:
-# - .info - An array of the information of that unit from Juju
-# - .file(PATH) - Get the details of a file on that unit
-# - .file_contents(PATH) - Get plain text output of PATH file from that unit
-# - .directory(PATH) - Get details of directory
-# - .directory_contents(PATH) - List files and folders in PATH on that unit
-# - .relation(relation, service:rel) - Get relation data from return service
-
-# Make sure to rename this file from something other than 00-autogen
+
+        try:
+            cls.deployment.setup(timeout=900)
+            cls.deployment.sentry.wait()
+        except amulet.helpers.TimeoutError:
+            amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
+        except:
+            raise
+
+    def test_case(self):
+        # Now you can use self.deployment.sentry.unit[UNIT] to address each of
+        # the units and perform more in-depth steps.  You can also reference
+        # the first unit as self.unit.
+        #
+        # There are three test statuses that can be triggered with
+        # amulet.raise_status():
+        #   - amulet.PASS
+        #   - amulet.FAIL
+        #   - amulet.SKIP
+        #
+        # Each unit has the following methods:
+        #   - .info - An array of the information of that unit from Juju
+        #   - .file(PATH) - Get the details of a file on that unit
+        #   - .file_contents(PATH) - Get plain text output of PATH file from that unit
+        #   - .directory(PATH) - Get details of directory
+        #   - .directory_contents(PATH) - List files and folders in PATH on that unit
+        #   - .relation(relation, service:rel) - Get relation data from return service
+        #          add tests here to confirm service is up and working properly
+        #
+        # For example, to confirm that it has a functioning HTTP server:
+        #
+        #     page = requests.get('http://{}'.format(self.unit.info['public-address']))
+        #     page.raise_for_status()
+        #
+        # More information on writing Amulet tests can be found at:
+        #
+        #     https://juju.ubuntu.com/docs/tools-amulet.html
+        pass
+
+
+if __name__ == '__main__':
+    unittest.main()

=== modified file 'charmtools/templates/tests/deploy.tpl'
--- charmtools/templates/tests/deploy.tpl	2013-12-10 17:37:20 +0000
+++ charmtools/templates/tests/deploy.tpl	2014-11-05 18:11:03 +0000
@@ -1,3 +1,3 @@
 #for $s in $services
-d.add('$s')
+        cls.deployment.add('$s')
 #end for

=== modified file 'charmtools/templates/tests/relate.tpl'
--- charmtools/templates/tests/relate.tpl	2013-12-10 21:03:14 +0000
+++ charmtools/templates/tests/relate.tpl	2014-11-05 18:11:03 +0000
@@ -1,3 +1,3 @@
 #for $r in $relations
-d.relate('$r[0]', '$r[1]')
+        cls.deployment.relate('$r[0]', '$r[1]')
 #end for

=== modified file 'tests/test_charm_generate.py'
--- tests/test_charm_generate.py	2014-02-10 17:21:40 +0000
+++ tests/test_charm_generate.py	2014-11-05 18:11:03 +0000
@@ -1,5 +1,8 @@
 import unittest
-from charmtools.generate import graph
+from charmtools.generate import (
+    graph,
+    copy_file,
+)
 from mock import patch
 
 
@@ -19,3 +22,15 @@
         self.assertEqual(graph('nointerface', 'requires'), None)
         m.search.assert_called_with({'series': 'precise',
                                      'provides': 'nointerface'})
+
+    @patch('charmtools.generate.Charm')
+    @patch('charmtools.generate.shutil')
+    def test_copy_file(self, msh, mcharm):
+        m = mcharm.return_value.is_charm.return_value = True
+        copy_file('1.ex', '/tmp')
+        msh.copy.assert_called()
+
+    @patch('charmtools.generate.Charm')
+    def test_not_charm(self, mcharm):
+        mcharm.return_value.is_charm.return_value = False
+        self.assertRaises(Exception, copy_file, '1.ex', '/no-charm')