yellow team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/charms/oneiric/buildbot-master/more-tests into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~benji/charms/oneiric/buildbot-master/more-tests/+merge/92123

This branch primarily adds a basic smoke test and fixes some bugs that
prevented the smoke test from passing.

- add a boostrap step to HACKING.txt
- change a (short circuiting) logical or into a bitwise
  (non-short-circuiting) or to fix a startup race condition
- add some log helpers for entering/exiting scripts
- spiff up the run() function so stderr doesn't leak to the console when
  running tests
- use a try/finally to be sure script exits are logged
- add a testing buildbot config

Tests:

Both test suites pass:

    python hooks/tests.py
    
    RESOLVE_TEST_CHARMS=1 tests/buildbot-master.test

-- 
https://code.launchpad.net/~benji/charms/oneiric/buildbot-master/more-tests/+merge/92123
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~benji/charms/oneiric/buildbot-master/more-tests into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== modified file 'HACKING.txt'
--- HACKING.txt	2012-02-02 18:35:04 +0000
+++ HACKING.txt	2012-02-08 20:09:18 +0000
@@ -14,7 +14,11 @@
    real juju executable, naming it "juju".  Edit the CHARM_TEST_REPO variable
    therein to reflect the location of the charm repo from step 1.
 
-3) Run the tests: RESOLVE_TEST_CHARMS=1 tests/buildbot-master.test
+3) Bootstrap the juju environment if not already done:
+
+    juju bootstrap
+
+4) Run the tests: RESOLVE_TEST_CHARMS=1 tests/buildbot-master.test
 
 
 Running the charm helper tests

=== modified file 'hooks/buildbot-relation-changed'
--- hooks/buildbot-relation-changed	2012-02-07 17:30:54 +0000
+++ hooks/buildbot-relation-changed	2012-02-08 20:09:18 +0000
@@ -5,6 +5,8 @@
 
 from helpers import (
     log,
+    log_entry,
+    log_exit,
     relation_get,
     relation_set,
     )
@@ -48,5 +50,8 @@
 
 
 if __name__ == '__main__':
-    log('BUILDBOT-RELATION-CHANGED HOOK:')
-    main()
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-02-08 09:57:46 +0000
+++ hooks/config-changed	2012-02-08 20:09:18 +0000
@@ -18,6 +18,8 @@
     get_config,
     install_extra_repository,
     log,
+    log_entry,
+    log_exit,
     run,
     )
 from local import (
@@ -65,6 +67,7 @@
 
     # Add a new repository if it was just added.
     added_or_changed = diff.added_or_changed
+
     if extra_repo and 'extra-repository' in added_or_changed:
         install_extra_repository(extra_repo)
         restart_required = True
@@ -124,6 +127,7 @@
 
 def initialize_buildbot(config):
     # Initialize the buildbot directory and (re)start buildbot.
+    log("Initializing buildbot")
     buildbot_dir =  config.get('installdir')
     master_cfg_path = os.path.join(buildbot_dir, 'master.cfg')
     shutil.move(master_cfg_path, master_cfg_path + '.original')
@@ -158,9 +162,8 @@
     if not os.path.exists(buildbot_dir):
         os.makedirs(buildbot_dir)
 
-    restart_required = (
-        handle_config_changes(config, diff) or
-        configure_buildbot(config, diff))
+    restart_required = handle_config_changes(config, diff)
+    restart_required |= configure_buildbot(config, diff)
 
     master_cfg_path = os.path.join(buildbot_dir, 'master.cfg')
     if restart_required and os.path.exists(master_cfg_path):
@@ -172,5 +175,8 @@
 
 
 if __name__ == '__main__':
-    log('CONFIG-CHANGED HOOK:')
-    main()
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-07 14:37:13 +0000
+++ hooks/helpers.py	2012-02-08 20:09:18 +0000
@@ -13,21 +13,39 @@
     'grep',
     'install_extra_repository',
     'log',
+    'log_entry',
+    'log_exit',
     'run',
     'relation_get',
     'relation_set',
     'unit_info',
     ]
 
+from textwrap import dedent
 import json
 import os
 import re
 import subprocess
 import sys
-from textwrap import dedent
+import time
 import yaml
 
 
+def run(*args):
+    """Run the command with the given arguments.
+
+    The first argument is the path to the command to run, subsequent arguments
+    are command-line arguments to be passed.
+    """
+    process = subprocess.Popen(args, stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE, close_fds=True)
+    stdout, stderr = process.communicate()
+    if process.returncode:
+        raise subprocess.CalledProcessError(
+            process.returncode, repr(args), output=stdout+stderr)
+    return stdout
+
+
 def command(*base_args):
     """Return a callable that will run the given command with any arguments.
 
@@ -48,7 +66,7 @@
     """
     def callable_command(*args):
         all_args = base_args + args
-        return subprocess.check_output(all_args, shell=False)
+        return run(*all_args)
 
     return callable_command
 
@@ -57,11 +75,6 @@
 apt_get_install = command('apt-get', 'install', '-y', '--force-yes')
 
 
-def run(*args):
-    log('run: ' + str(args))
-    return subprocess.check_output(args, shell=False)
-
-
 def get_config():
     config_get = command('config-get', '--format=json')
     return json.loads(config_get())
@@ -72,9 +85,9 @@
         run('apt-add-repository', extra_repository)
         run('apt-get', 'update')
     except subprocess.CalledProcessError as e:
-        log("Error adding repository %s" % extra_repository)
+        log('Error adding repository: ' + extra_repository)
         log(e)
-        sys.exit(-1)
+        raise
 
 
 def relation_get(*args):
@@ -108,6 +121,18 @@
     return line.split('=')[1].strip('"\' ')
 
 
+def script_name():
+    return os.path.basename(sys.argv[0])
+
+
+def log_entry():
+    log("<-- Entering {}".format(script_name()))
+
+
+def log_exit():
+    log("--> Exiting {}".format(script_name()))
+
+
 class DictDiffer:
     """
     Calculate the difference between two dictionaries as:
@@ -158,6 +183,7 @@
             new[k] = self.current_dict.get(k)
             old[k] = self.past_dict.get(k)
         return "%s -> %s" % (old, new)
+
     def __str__(self):
         if self.modified:
             s = dedent("""\
@@ -174,6 +200,28 @@
         return s
 
 
+def unit_info(service_name, item_name, data=None):
+    if data is None:
+        data = yaml.safe_load(run('juju', 'status'))
+    services = data['services'][service_name]
+    units = services['units']
+    item = units.items()[0][1][item_name]
+    return item
+
+
+def wait_for_unit(service_name, timeout=120):
+    start_time = time.time()
+    while True:
+        status = unit_info(service_name, 'state')
+        if 'error' in status or status == 'started':
+            break
+        if time.time() - start_time >= timeout:
+            raise RuntimeError('timeout waiting for service to start')
+        time.sleep(0.1)
+    state = unit_info('buildbot-master', 'state')
+    if state != 'started':
+        raise RuntimeError('unit did not start, state: ' + state)
+
 class Serializer:
     """Handle JSON (de)serialization."""
 

=== modified file 'hooks/install'
--- hooks/install	2012-02-07 17:30:54 +0000
+++ hooks/install	2012-02-08 20:09:18 +0000
@@ -11,6 +11,8 @@
     apt_get_install,
     get_config,
     log,
+    log_entry,
+    log_exit,
     run,
     )
 from local import (
@@ -44,5 +46,8 @@
 
 
 if __name__ == '__main__':
-    log('INSTALL HOOK:')
-    main()
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/local.py'
--- hooks/local.py	2012-02-07 14:34:40 +0000
+++ hooks/local.py	2012-02-08 20:09:18 +0000
@@ -11,6 +11,7 @@
     'config_json',
     'create_slave',
     'generate_string',
+    'HTTP_PORT_PROTOCOL',
     'slave_json',
     ]
 
@@ -26,6 +27,9 @@
     )
 
 
+HTTP_PORT_PROTOCOL = '8010/TCP'
+
+
 def _get_buildbot_dir():
     config = get_config()
     return config.get('installdir')

=== modified file 'hooks/start'
--- hooks/start	2012-02-07 13:29:08 +0000
+++ hooks/start	2012-02-08 20:09:18 +0000
@@ -5,15 +5,21 @@
 
 from helpers import (
     log,
+    log_entry,
+    log_exit,
     run,
     )
+from local import HTTP_PORT_PROTOCOL
 
 
 def main():
     # Actually starting the buildbot happens in config-changed.
-    run('open-port', '8010/TCP')
+    run('open-port', HTTP_PORT_PROTOCOL)
 
 
 if __name__ == '__main__':
-    log('START HOOK:')
-    main()
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/stop'
--- hooks/stop	2012-02-07 13:29:08 +0000
+++ hooks/stop	2012-02-08 20:09:18 +0000
@@ -6,18 +6,24 @@
 from helpers import (
     get_config,
     log,
+    log_entry,
+    log_exit,
     run,
     )
+from local import HTTP_PORT_PROTOCOL
 
 
 def main():
     config = get_config()
     log('Stopping buildbot in {}'.format(config['installdir']))
-    run('close-port', '8010/TCP')
+    run('close-port', HTTP_PORT_PROTOCOL)
 
     log('Finished stopping buildbot')
 
 
 if __name__ == '__main__':
-    log('STOP HOOK:')
-    main()
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/tests.py'
--- hooks/tests.py	2012-02-07 14:37:13 +0000
+++ hooks/tests.py	2012-02-08 20:09:18 +0000
@@ -3,11 +3,36 @@
 from subprocess import CalledProcessError
 
 from helpers import (
+    DictDiffer,
     command,
-    DictDiffer,
+    run,
     unit_info,
     )
 
+from subprocess import CalledProcessError
+
+class TestRun(unittest.TestCase):
+
+    def testSimpleCommand(self):
+        # Running a simple command (ls) works and running the command
+        # produces a string.
+        self.assertIsInstance(run('/bin/ls'), str)
+
+    def testStdoutReturned(self):
+        # Running a simple command (ls) works and running the command
+        # produces a string.
+        self.assertIn('Usage:', run('/bin/ls', '--help'))
+
+    def testSimpleCommand(self):
+        # If an error ocurrs a CalledProcessError is raised with the return
+        # code, command executed, and the output of the command.
+        with self.assertRaises(CalledProcessError) as info:
+            run('ls', '--not a valid switch')
+        exception = info.exception
+        self.assertEqual(2, exception.returncode)
+        self.assertEqual("('ls', '--not a valid switch')", exception.cmd)
+        self.assertIn('unrecognized option', exception.output)
+
 
 class TestCommand(unittest.TestCase):
 

=== modified file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	2012-02-07 12:35:06 +0000
+++ tests/buildbot-master.test	2012-02-08 20:09:18 +0000
@@ -1,27 +1,42 @@
 #!/usr/bin/python
-
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-from helpers import command, run, unit_info
+from helpers import command, run, unit_info, wait_for_unit
+import base64
+import os.path
 import time
 import unittest
+import urllib2
 
 juju = command('juju')
 
 class testCharm(unittest.TestCase):
 
+    def tearDown(self):
+        juju('destroy-service', 'buildbot-master')
+
     def testDeploy(self):
+        # See if the charm will actuall deploy correctly.  This test may be
+        # made redunant by standard charm tests run by some future centralized
+        # charm repository.  
+        juju('deploy', 'buildbot-master')
+        wait_for_unit('buildbot-master')
+        self.assertEqual(unit_info('buildbot-master', 'state'), 'started')
+
+    def testPortOpened(self):
+        juju('deploy', 'buildbot-master')
+        juju('set', 'buildbot-master', 'extra-packages=git')
+        config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
+        config = base64.b64encode(open(config_path).read())
+        juju('set', 'buildbot-master', 'config-file='+config)
+        wait_for_unit('buildbot-master')
+        addr = unit_info('buildbot-master', 'public-address')
         try:
-            juju('deploy', 'buildbot-master')
-            while True:
-                status = unit_info('buildbot-master', 'state')
-                if 'error' in status or status == 'started':
-                    break
-                time.sleep(0.1)
-            self.assertEqual(unit_info('buildbot-master', 'state'), 'started')
-        finally:
-            juju('destroy-service', 'buildbot-master')
+            page = urllib2.urlopen('http://{}:8010'.format(addr)).read()
+        except urllib2.URLError:
+            self.fail('could not fetch buildbot master status page')
+        self.assertIn('Welcome to the Buildbot', page)
 
 
 if __name__ == '__main__':

=== added file 'tests/test.cfg'
--- tests/test.cfg	1970-01-01 00:00:00 +0000
+++ tests/test.cfg	2012-02-08 20:09:18 +0000
@@ -0,0 +1,118 @@
+# -*- python -*-
+# ex: set syntax=python:
+
+# This is a sample buildmaster config file. It must be installed as
+# 'master.cfg' in your buildmaster's base directory.
+
+# This is the dictionary that the buildmaster pays attention to. We also use
+# a shorter alias to save typing.
+c = BuildmasterConfig = {}
+
+####### BUILDSLAVES
+
+# The 'slaves' list defines the set of recognized buildslaves. Each element is
+# a BuildSlave object, specifying a username and password.  The same username and
+# password must be configured on the slave.
+from buildbot.buildslave import BuildSlave
+c['slaves'] = []
+
+# 'slavePortnum' defines the TCP port to listen on for connections from slaves.
+# This must match the value configured into the buildslaves (with their
+# --master option)
+c['slavePortnum'] = 9989
+
+####### CHANGESOURCES
+
+# the 'change_source' setting tells the buildmaster how it should find out
+# about source code changes.  Here we point to the buildbot clone of pyflakes.
+
+from buildbot.changes.gitpoller import GitPoller
+c['change_source'] = GitPoller(
+        'git://github.com/buildbot/pyflakes.git',
+        branch='master', pollinterval=1200)
+
+####### SCHEDULERS
+
+# Configure the Schedulers, which decide how to react to incoming changes.  In this
+# case, just kick off a 'runtests' build
+
+from buildbot.scheduler import Scheduler
+c['schedulers'] = []
+c['schedulers'].append(Scheduler(name="all", branch=None,
+                                 treeStableTimer=None,
+                                 builderNames=["runtests"]))
+
+####### BUILDERS
+
+# The 'builders' list defines the Builders, which tell Buildbot how to perform a build:
+# what steps, and which slaves can execute them.  Note that any particular build will
+# only take place on one slave.
+
+from buildbot.process.factory import BuildFactory
+from buildbot.steps.source import Git
+from buildbot.steps.shell import ShellCommand
+
+factory = BuildFactory()
+# check out the source
+factory.addStep(Git(repourl='git://github.com/buildbot/pyflakes.git', mode='copy'))
+# run the tests (note that this will require that 'trial' is installed)
+factory.addStep(ShellCommand(command=["trial", "pyflakes"]))
+
+from buildbot.config import BuilderConfig
+
+c['builders'] = [
+    BuilderConfig(name="runtests",
+      # Buildbot enforces that the slavenames list must not be empty. Our
+      # wrapper will remove the empty string and replace it with proper values.
+      slavenames=[''],
+      factory=factory),
+    ]
+
+####### STATUS TARGETS
+
+# 'status' is a list of Status Targets. The results of each build will be
+# pushed to these targets. buildbot/status/*.py has a variety to choose from,
+# including web pages, email senders, and IRC bots.
+
+c['status'] = []
+
+from buildbot.status import html
+from buildbot.status.web import auth, authz
+authz_cfg=authz.Authz(
+    # change any of these to True to enable; see the manual for more
+    # options
+    gracefulShutdown = False,
+    forceBuild = True, # use this to test your slave once it is set up
+    forceAllBuilds = False,
+    pingBuilder = False,
+    stopBuild = False,
+    stopAllBuilds = False,
+    cancelPendingBuild = False,
+)
+c['status'].append(html.WebStatus(http_port=8010, authz=authz_cfg))
+
+####### PROJECT IDENTITY
+
+# the 'projectName' string will be used to describe the project that this
+# buildbot is working on. For example, it is used as the title of the
+# waterfall HTML page. The 'projectURL' string will be used to provide a link
+# from buildbot HTML pages to your project's home page.
+
+c['projectName'] = "Pyflakes"
+c['projectURL'] = "http://divmod.org/trac/wiki/DivmodPyflakes";
+
+# the 'buildbotURL' string should point to the location where the buildbot's
+# internal web server (usually the html.WebStatus page) is visible. This
+# typically uses the port number set in the Waterfall 'status' entry, but
+# with an externally-visible host name which the buildbot cannot figure out
+# without some help.
+
+c['buildbotURL'] = "http://localhost:8010/";
+
+####### DB URL
+
+# This specifies what database buildbot uses to store change and scheduler
+# state.  You can leave this at its default for all but the largest
+# installations.
+c['db_url'] = "sqlite:///state.sqlite"
+