yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/oneiric/buildbot-slave/tests_again 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/tests_again/+merge/92256

- buildslave charm tests refactoring
- using run function to chmod the script in install hook
- changed symlinks (helpers.py, tests.cfg)
-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-slave/tests_again/+merge/92256
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/charms/oneiric/buildbot-slave/tests_again into lp:~yellow/charms/oneiric/buildbot-slave/trunk.

=== modified file 'hooks/install'
--- hooks/install	2012-02-09 10:05:06 +0000
+++ hooks/install	2012-02-09 11:50:30 +0000
@@ -10,6 +10,7 @@
     log,
     log_entry,
     log_exit,
+    run,
     )
 import os
 import shlex
@@ -70,8 +71,7 @@
     log('Retrieving script: {}.'.format(url))
     script = retrieve(url, path)
     log('Changing script mode.')
-    chmod = command('chmod')
-    chmod('+x', script)
+    run('chmod', '+x', script)
     log('Executing script: {}.'.format(script))
     return subprocess.call([script] + shlex.split(str(args)))
 

=== modified file 'tests/buildbot-slave.test'
--- tests/buildbot-slave.test	2012-02-08 18:00:09 +0000
+++ tests/buildbot-slave.test	2012-02-09 11:50:30 +0000
@@ -12,6 +12,8 @@
     command,
     encode_file,
     unit_info,
+    wait_for_unit,
+    wait_for_relation,
     )
 from openport import (
     PORT,
@@ -19,12 +21,6 @@
     )
 
 
-# To run tests, having juju bootstrapped:
-# JUJU_REPOSITORY=. oneiric/buildbot-slave/tests/buildbot-slave.test
-# XXX 2012-02-08 gmb:
-#     This needs reducing; it's set high for debugging purposes only.
-MAX_ATTEMPTS = 600
-
 juju = command('juju')
 
 
@@ -35,10 +31,15 @@
         self.repository = os.getenv('JUJU_REPOSITORY')
         self.environment = os.getenv('JUJU_ENVIRONMENT')
         self.tests_dir = os.path.dirname(os.path.abspath(__file__))
-
-    def deploy(self, config=None, sleep_time=0.1, charm_name=None):
-        deploy_name = charm_name = charm_name or self.charm_name
+        self.started_services = set()
+
+    def tearDown(self):
+        for service_name in self.started_services:
+            juju('destroy-service', service_name)
+
+    def deploy(self, service_name, config=None):
         args = ['deploy']
+        deploy_name = service_name
         if self.repository is not None:
             args.append('--repository=' + self.repository)
             deploy_name = 'local:' + deploy_name
@@ -48,24 +49,23 @@
             args.append('--config=' + config)
         args.append(deploy_name)
         juju(*args)
-        status_attempts = 0
-        while status_attempts < MAX_ATTEMPTS:
-            status = unit_info(charm_name, 'state')
-            if 'error' in status:
-                return False
-            if status == 'started':
-                return True
-            time.sleep(sleep_time)
-            status_attempts += 1
-        raise Exception("Charm took too long to deploy.")
+        self.started_services.add(service_name)
+        wait_for_unit(service_name, timeout=600)
+
+    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.
-        try:
-            started = self.deploy()
-            self.assertTrue(started)
-        finally:
-            juju('destroy-service', self.charm_name)
+        self.deploy(self.charm_name)
+        self.assertEqual('started', unit_info(self.charm_name, 'state'))
 
     def do_not_test_script(self):
         # DISABLED BECAUSE IT FAILS ALL THE TIME.
@@ -87,44 +87,31 @@
 
     def test_master_slave_relationship(self):
         master_charm_name = 'buildbot-master'
+        self.deploy(master_charm_name)
+        self.deploy(self.charm_name)
+        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')
+        # XXX 2012-02-08 gmb
+        #     This avoids us trying to check whether the slave
+        #     is connected until the relation is initialized,
+        #     but... (see next XXX)
+        self.add_relation('buildbot', self.charm_name, master_charm_name)
+        # XXX 2012-02-08 gmb:
+        #     This still sometimes gets called _before_ the
+        #     buildslave connects to the buildbot master. Hence
+        #     this rather ugly sleep here.
+        time.sleep(5)
+        url = self.get_url(master_charm_name, 8010, '/buildslaves')
         try:
-            self.deploy(charm_name=master_charm_name)
-            self.deploy()
-            juju('set', master_charm_name, 'extra-packages=git')
-            encoded_config = encode_file(
-                os.path.join(self.tests_dir, '..', 'examples', 'master.cfg'))
-            juju(
-                'set', master_charm_name,
-                'config-file={}'.format(encoded_config))
-            juju('expose', master_charm_name)
-            juju('set', self.charm_name, 'builders=runtests')
-            juju('add-relation', self.charm_name, master_charm_name)
-            address = unit_info(master_charm_name, 'public-address')
-            url = 'http://{}:{}/buildslaves'.format(address, 8010)
-            # Wait for buildbot to restart, since there's a
-            # potential race condition with an asynchronous service.
-            while True:
-                # XXX 2012-02-08 gmb
-                #     This avoids us trying to check whether the slave
-                #     is connected until the relation is initialized,
-                #     but... (see next XXX)
-                relation_info = unit_info(self.charm_name, 'relations')
-                if relation_info['buildbot']['state'] == 'up':
-                    break
-                time.sleep(0.1)
-            try:
-                # XXX 2012-02-08 gmb:
-                #     This still sometimes gets called _before_ the
-                #     buildslave connects to the buildbot master. Hence
-                #     this rather ugly sleep here.
-                time.sleep(5)
-                stream = urllib2.urlopen(url)
-            except urllib2.HTTPError:
-                self.fail('Unable to connect to {}.'.format(url))
-            self.assertIn('Idle', stream.read())
-        finally:
-            juju('destroy-service', self.charm_name)
-            juju('destroy-service', master_charm_name)
+            stream = urllib2.urlopen(url)
+        except urllib2.HTTPError:
+            self.fail('Unable to connect to {}.'.format(url))
+        self.assertIn('Idle', stream.read())
+
 
 if __name__ == '__main__':
     unittest.main()

=== modified symlink 'tests/helpers.py'
=== target changed u'../../buildbot-master/hooks/helpers.py' => u'../hooks/helpers.py'
=== added symlink 'tests/test.cfg'
=== target is u'../../buildbot-master/tests/test.cfg'