yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/oneiric/buildbot-slave/02-13 into lp:~yellow/charms/oneiric/buildbot-slave/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-slave/02-13/+merge/92813

*Changes*:

- Tests requiring buildbot master no longer use base64 encoded configuration.

- Added a test for existing slave joining a brand new master.

- Added a test for broken relationship between the slave and the master.

- Refactoring to be DRY.


*Tests*:

Tests are run using ec2 environment and the master branch that can be found here:
lp:~frankban/charms/oneiric/buildbot-master/02-13::

    $ RESOLVE_TEST_CHARMS=1 tests/buildbot-slave.test -v
    test_deploy (__main__.TestCharm) ... ok
    test_master_slave_relationship (__main__.TestCharm) ... ok
    test_relation_broken (__main__.TestCharm) ... ok
    test_script (__main__.TestCharm) ... ok
    test_script_args_formatting (__main__.TestCharm) ... ok
    test_slave_with_new_master (__main__.TestCharm) ... ok

    ----------------------------------------------------------------------
    Ran 6 tests in 904.867s

    OK

-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-slave/02-13/+merge/92813
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/charms/oneiric/buildbot-slave/02-13 into lp:~yellow/charms/oneiric/buildbot-slave/trunk.

=== modified file 'tests/buildbot-slave.test'
--- tests/buildbot-slave.test	2012-02-10 20:08:16 +0000
+++ tests/buildbot-slave.test	2012-02-13 18:05:21 +0000
@@ -8,7 +8,6 @@
 
 from helpers import (
     command,
-    encode_file,
     make_charm_config_file,
     unit_info,
     wait_for_page_contents,
@@ -28,6 +27,7 @@
 
     def setUp(self):
         self.charm_name = 'buildbot-slave'
+        self.master_charm_name = 'buildbot-master'
         self.environment = os.getenv('JUJU_ENVIRONMENT')
         self.started_services = set()
 
@@ -35,6 +35,18 @@
         for service_name in self.started_services:
             juju('destroy-service', service_name)
 
+    def add_relation(self, relation, service1, service2):
+        juju('add-relation',
+             '{}:{}'.format(service1, relation),
+             '{}:{}'.format(service2, relation))
+        wait_for_relation(service1, relation)
+
+    def check_relation(self):
+        self.add_relation('buildbot', self.charm_name, self.master_charm_name)
+        url = self.get_url(self.master_charm_name, 8010, '/buildslaves')
+        wait_for_page_contents(url, 'buildbot-slave/')
+        wait_for_page_contents(url, 'Idle')
+
     def deploy(self, service_name, config=None):
         args = ['deploy', service_name]
         if self.environment is not None:
@@ -45,46 +57,24 @@
         juju(*args)
         self.started_services.add(service_name)
 
-    def add_relation(self, relation, service1, service2):
-        juju('add-relation',
-             '{}:{}'.format(service1, relation),
-             '{}:{}'.format(service2, relation))
-        wait_for_relation(service1, relation)
-
-    def get_url(self, service_name, port, path=''):
-        address = unit_info(service_name, 'public-address')
-        return 'http://{}:{}{}'.format(address, port, path)
-
-    def test_deploy(self):
-        # Ensure the charm starts correctly when deployed.
-        self.deploy(self.charm_name)
-        # XXX 2012-02-10 frankban:
-        #     We increase the timeout to 10 minutes to avoid runtime errors
-        #     when a charm is deployed and the corresponding instance
-        #     is not already created.
-        wait_for_unit(self.charm_name, timeout=600)
-        self.assertEqual('started', unit_info(self.charm_name, 'state'))
-
-    def test_master_slave_relationship(self):
-        # Ensure the master-slave relationship is correctly established.
-        master_charm_name = 'buildbot-master'
+    def deploy_master(self):
+        config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
+        master_config = {self.master_charm_name: {
+            'extra-packages': 'git',
+            'config-file': open(config_path).read(),
+            }}
+        self.deploy(self.master_charm_name, master_config)
+        juju('expose', self.master_charm_name)
+
+    def deploy_both(self):
         # We deploy both and then wait because it's quite a bit faster.
-        self.deploy(master_charm_name)
+        self.deploy_master()
         self.deploy(self.charm_name)
         # XXX 2012-02-10 frankban:
-        #     See the above XXX comment in `test_deploy()`.
-        wait_for_unit(master_charm_name, timeout=600)
+        #     See the below XXX comment in `test_deploy()`.
+        wait_for_unit(self.master_charm_name, timeout=600)
         wait_for_unit(self.charm_name, timeout=600)
-        juju('set', master_charm_name, 'extra-packages=git')
-        config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
-        config = encode_file(config_path)
-        juju('set', 'buildbot-master', 'config-file='+config)
-        juju('expose', master_charm_name)
         juju('set', self.charm_name, 'builders=runtests')
-        self.add_relation('buildbot', self.charm_name, master_charm_name)
-        url = self.get_url(master_charm_name, 8010, '/buildslaves')
-        wait_for_page_contents(url, 'buildbot-slave/')
-        wait_for_page_contents(url, 'Idle')
 
     def get_script_config(self, service, options=None):
         if options is None:
@@ -95,7 +85,7 @@
                 'http://bazaar.launchpad.net/~yellow/charms/'
                 'oneiric/buildbot-slave/trunk/tests/create_file.py',
             'script-path': 'create_file.py',
-        })
+            })
         return {service: options}
 
     def get_ssh_command(self, service):
@@ -110,6 +100,43 @@
             '--',
             )
 
+    def get_url(self, service_name, port, path=''):
+        address = unit_info(service_name, 'public-address')
+        return 'http://{}:{}{}'.format(address, port, path)
+
+    def test_deploy(self):
+        # Ensure the charm starts correctly when deployed.
+        self.deploy(self.charm_name)
+        # XXX 2012-02-10 frankban:
+        #     We increase the timeout to 10 minutes to avoid runtime errors
+        #     when a charm is deployed and the corresponding instance
+        #     is not already created.
+        wait_for_unit(self.charm_name, timeout=600)
+        self.assertEqual('started', unit_info(self.charm_name, 'state'))
+
+    def test_master_slave_relationship(self):
+        # Ensure the master-slave relationship is correctly established.
+        self.deploy_both()
+        self.check_relation()
+
+    def test_slave_with_new_master(self):
+        # Ensure a relation can be correctly established between a previously
+        # used slave and a brand new master.
+        self.deploy_both()
+        self.add_relation('buildbot', self.charm_name, self.master_charm_name)
+        juju('destroy-service', self.master_charm_name)
+        self.deploy_master()
+        self.check_relation()
+
+    def test_relation_broken(self):
+        # Ensure the master correctly handles broken relations.
+        self.deploy_both()
+        self.add_relation('buildbot', self.charm_name, self.master_charm_name)
+        juju('remove-relation', self.charm_name, self.master_charm_name)
+        url = self.get_url(self.master_charm_name, 8010, '/buildslaves')
+        wait_for_page_contents(
+            url, 'Idle', validate=lambda page, s: s not in page)
+
     def test_script(self):
         # Ensure the script is run when the charm is deployed.
         config = self.get_script_config(self.charm_name)
@@ -128,7 +155,7 @@
         options = {
             'installdir': installdir,
             'script-args': '{installdir}'
-        }
+            }
         config = self.get_script_config(self.charm_name, options)
         self.deploy(self.charm_name, config=config)
         # XXX 2012-02-10 frankban: