yellow team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/charms/oneiric/buildbot-slave/who-needs-sleep into lp:~yellow/charms/oneiric/buildbot-slave/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~benji/charms/oneiric/buildbot-slave/who-needs-sleep/+merge/92298

This branch makes the test_master_slave_relationship test more resilient (removing a sleep) and much faster (by parallelizing unit startup.

I noticed that the (do_not_)test_script test won't run any more (it doesn't pass a required argument to .deploy()) so I removed it.  It's safe in Bazaar's Bosom should we need to call upon it.
-- 
https://code.launchpad.net/~benji/charms/oneiric/buildbot-slave/who-needs-sleep/+merge/92298
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~benji/charms/oneiric/buildbot-slave/who-needs-sleep into lp:~yellow/charms/oneiric/buildbot-slave/trunk.

=== modified file 'hooks/tests.py'
--- hooks/tests.py	2012-02-07 13:56:40 +0000
+++ hooks/tests.py	1970-01-01 00:00:00 +0000
@@ -1,51 +0,0 @@
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-import unittest
-
-from helpers import command
-from subprocess import CalledProcessError
-
-
-class TestCommand(unittest.TestCase):
-
-    def test_simple_command(self):
-        # Creating a simple command (ls) works and running the command
-        # produces a string.
-        ls = command('/bin/ls')
-        self.assertIsInstance(ls(), str)
-
-    def test_arguments(self):
-        # Arguments can be passed to commands.
-        ls = command('/bin/ls')
-        self.assertIn('Usage:', ls('--help'))
-
-    def test_missing_executable(self):
-        # If the command does not exist, an OSError (No such file or
-        # directory) is raised.
-        bad = command('this command does not exist')
-        with self.assertRaises(OSError) as info:
-            bad()
-        self.assertEqual(2, info.exception.errno)
-
-    def test_error(self):
-        # If the command returns a non-zero exit code, an exception is raised.
-        ls = command('/bin/ls')
-        with self.assertRaises(CalledProcessError):
-            ls('--not a valid switch')
-
-    def test_baked_in_arguments(self):
-        # Arguments can be passed when creating the command as well as when
-        # executing it.
-        ll = command('/bin/ls', '-l')
-        self.assertIn('rw', ll()) # Assumes a file is r/w in the pwd.
-        self.assertIn('Usage:', ll('--help'))
-
-    def test_quoting(self):
-        # There is no need to quote special shell characters in commands.
-        ls = command('/bin/ls')
-        ls('--help', '>')
-
-
-if __name__ == '__main__':
-    unittest.main()

=== target is u'../../buildbot-master/hooks/tests.py'
=== modified file 'tests/buildbot-slave.test'
--- tests/buildbot-slave.test	2012-02-09 12:06:33 +0000
+++ tests/buildbot-slave.test	2012-02-09 15:55:30 +0000
@@ -6,14 +6,14 @@
 import os
 import time
 import unittest
-import urllib2
 
 from helpers import (
     command,
     encode_file,
     unit_info,
+    wait_for_page_contents,
+    wait_for_relation,
     wait_for_unit,
-    wait_for_relation,
     )
 from openport import (
     PORT,
@@ -56,7 +56,6 @@
         #     (e.g. just after `juju bootstrap`).
         #     We should try to wait for unit in a smarter way
         #     (maybe checking for "pending"?).
-        wait_for_unit(service_name, timeout=600)
 
     def add_relation(self, relation, service1, service2):
         juju('add-relation',
@@ -71,52 +70,26 @@
     def test_deploy(self):
         # Ensure the charm starts correctly when deployed.
         self.deploy(self.charm_name)
+        wait_for_unit(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.
-        # Ensure a script supplied during deploy is correctly run.
-        config = os.path.join(self.tests_dir, 'config.test.yaml')
-        try:
-            started = self.deploy(config=config)
-            self.assertTrue(started)
-            address = unit_info(self.charm_name, 'public-address')
-            url = 'http://{}:{}'.format(address, PORT)
-            try:
-                stream = urllib2.urlopen(url)
-            except urllib2.HTTPError:
-                self.fail('Unable to connect to {}.'.format(url))
-            self.assertEqual(TEXT, stream.read())
-        finally:
-#            juju('destroy-service', self.charm_name)
-            pass
-
     def test_master_slave_relationship(self):
         master_charm_name = 'buildbot-master'
+        # We deploy both and then wait because it's quite a bit faster.
         self.deploy(master_charm_name)
         self.deploy(self.charm_name)
+        wait_for_unit(master_charm_name)
+        wait_for_unit(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:
-            stream = urllib2.urlopen(url)
-        except urllib2.HTTPError:
-            self.fail('Unable to connect to {}.'.format(url))
-        self.assertIn('Idle', stream.read())
+        wait_for_page_contents(url, 'buildbot-slave/')
+        wait_for_page_contents(url, 'Idle')
 
 
 if __name__ == '__main__':