Merge lp:~benji/charms/oneiric/buildbot-master/more-tests into lp:~yellow/charms/oneiric/buildbot-master/trunk

[Benji York]

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

[Brad Crittenden (bac)]

Hi Benji,

Some of this code already landed in my branch. Hope that doesn't complicate things.

Also, the branch I landed for gmb and frankban made changes to unit_info. You may want to look at what is in trunk before attempting the merge.

As we discussed this morning there is a move afoot to follow more of PEP-8 than LP coding style so perhaps you could update the tests to getRidOfCamelCase in_favor_of_underscores?

typo: occurs not ocurrs

redundant not redunant

There is now an 'encode_file' helper if you want to use it.

review: Approve (code)

[lp:~benji/charms/oneiric/buildbot-master/more-tests]

20. By Benji York on 2012-02-08

fix spelling errors

21. By Benji York on 2012-02-08

merge from trunk and incorporate some review suggestions

22. By Benji York on 2012-02-08

simplify

23. By Benji York on 2012-02-08

use the encode_file helper

[Benji York (benji)]

> Also, the branch I landed for gmb and frankban made changes to unit_info. You
> may want to look at what is in trunk before attempting the merge.

Thanks for the heads-up. I was able to merge without too much trouble.

> As we discussed this morning there is a move afoot to follow more of PEP-8
> than LP coding style so perhaps you could update the tests to
> getRidOfCamelCase in_favor_of_underscores?

Good, I like underscores better. Done.

> typo: occurs not ocurrs
>
> redundant not redunant

Thanks. Fixed.

> There is now an 'encode_file' helper if you want to use it.

That feels like taking the helpers a bit far, but if we already have it,
I guess I'll follow along.

=== modified file 'HACKING.txt'
--- HACKING.txt	2012-02-02 18:35:04 +0000
+++ HACKING.txt	2012-02-08 22:09:17 +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/helpers.py'
--- hooks/helpers.py	2012-02-08 20:56:21 +0000
+++ hooks/helpers.py	2012-02-08 22:09:17 +0000
@@ -21,6 +21,7 @@
     'unit_info',
     ]
 
+from textwrap import dedent
 import base64
 import json
 import os
@@ -28,10 +29,25 @@
 import StringIO
 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.
 
@@ -52,7 +68,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
 
@@ -61,10 +77,6 @@
 apt_get_install = command('apt-get', 'install', '-y', '--force-yes')
 
 
-def run(*args):
-    return subprocess.check_output(args, shell=False)
-
-
 def get_config():
     config_get = command('config-get', '--format=json')
     return json.loads(config_get())
@@ -75,9 +87,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):
@@ -126,16 +138,16 @@
     return line.split('=')[1].strip('"\' ')
 
 
-def _script_name():
+def script_name():
     return os.path.basename(sys.argv[0])
 
 
 def log_entry():
-    log("<-- Entering {}".format(_script_name()))
+    log("<-- Entering {}".format(script_name()))
 
 
 def log_exit():
-    log("--> Exiting {}".format(_script_name()))
+    log("--> Exiting {}".format(script_name()))
 
 
 class DictDiffer:
@@ -188,6 +200,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("""\
@@ -204,6 +217,27 @@
         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:
+        state = unit_info(service_name, 'state')
+        if 'error' in state or state == 'started':
+            break
+        if time.time() - start_time >= timeout:
+            raise RuntimeError('timeout waiting for service to start')
+        time.sleep(0.1)
+    if state != 'started':
+        raise RuntimeError('unit did not start, state: ' + state)
+
 class Serializer:
     """Handle JSON (de)serialization."""
 
 
=== modified file 'hooks/tests.py'
--- hooks/tests.py	2012-02-07 14:37:13 +0000
+++ hooks/tests.py	2012-02-08 22:09:17 +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 occurs 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 22:09:17 +0000
@@ -1,27 +1,47 @@
 #!/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
-import time
+from helpers import (
+    command,
+    encode_file,
+    run,
+    unit_info,
+    wait_for_unit,
+    )
+import base64
+import os.path
 import unittest
+import urllib2
 
 juju = command('juju')
 
-class testCharm(unittest.TestCase):
-
-    def testDeploy(self):
+class TestCharm(unittest.TestCase):
+
+    def tearDown(self):
+        juju('destroy-service', 'buildbot-master')
+
+    def test_deploy(self):
+        # See if the charm will actual deploy correctly.  This test may be
+        # made redundant 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 test_port_opened(self):
+        juju('deploy', 'buildbot-master')
+        juju('set', 'buildbot-master', '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)
+        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 22:09:17 +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"
+