Merge lp:~gmb/charms/oneiric/buildbot-slave/charm-tests into lp:~yellow/charms/oneiric/buildbot-slave/trunk

[Graham Binns]

Changes from gmb and frankban 2012-02-08:
=========================================

 - We've added new, Python tests to the slave.
 - We have two working tests:
   - A test that the slave deploys successfully.
   - A test that the relation is set up correctly*
 - We have a test for deploying the slave with a script, but that currently fails all the time and we can't figure out why; it's disabled for now.

We've had to add some significant time.sleep() calls in order to avoid race conditions, particularly in the relationship test, since Juju does everything asynchronously and you can't rely on it actually having done things until you've checked juju status. I've added XXXs for these where appropriate.

[Benji York (benji)]

This branch looks good. Here are the suggestions I had:

Instead of creating a command to just call it once (starting on
line 15 of the diff):

    - command('chmod', '+x', script)
    + chmod = command('chmod')
    + chmod('+x', script)

I suggest using "run":

    run('chmod', '+x', script)

Instead of doing your own wait-for-the-unit-to-come-up loop in
buildbot-slave.test you can use helpers.wait_for_unit().

We could use a HACKING.txt file like the master.

Regarding (do_not_)test_script: If I'm understanding correctly the
function of the script configuration options, we could test them without
going as far as the openport.py script goes. For example, we could have
a well-known file (say /tmp/the-slave-setup-script-works) that we verify
does not exist, run a deploy who's configuration specifies a test script
to create the file, and then verify that the file exists.

review: Approve (code)

=== modified file '.bzrignore'
--- .bzrignore	2012-02-07 19:39:04 +0000
+++ .bzrignore	2012-02-08 18:09:23 +0000
@@ -1,1 +1,2 @@
 revision
+examples

=== modified file 'hooks/install'
--- hooks/install	2012-02-07 15:55:26 +0000
+++ hooks/install	2012-02-08 18:09:23 +0000
@@ -68,7 +68,8 @@
     log('Retrieving script: {}.'.format(url))
     script = retrieve(url, path)
     log('Changing script mode.')
-    command('chmod', '+x', script)
+    chmod = command('chmod')
+    chmod('+x', script)
     log('Executing script: {}.'.format(script))
     return subprocess.call([script] + shlex.split(str(args)))
 

=== removed file 'tests/100_buildbot-slave.config.test'
--- tests/100_buildbot-slave.config.test	2012-02-07 12:33:51 +0000
+++ tests/100_buildbot-slave.config.test	1970-01-01 00:00:00 +0000
@@ -1,72 +0,0 @@
-#!/bin/sh
-
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-set -e
-
-teardown() {
-    juju destroy-service buildbot-slave
-    if [ -n "$datadir" ] ; then
-        if [ -f $datadir/passed ]; then
-            rm -r $datadir
-        else
-            echo $datadir preserved
-        fi
-    fi
-}
-trap teardown EXIT
-
-juju deploy --repository=$PWD/../ local:buildbot-slave
-juju expose buildbot-slave
-
-for try in `seq 1 600` ; do
-    slave_host=`juju status | tests/get-unit-info buildbot-slave public-address`
-    if [ -z "$slave_host" ] ; then
-        sleep 1
-    else
-        break
-    fi
-done
-
-if [ -z "$slave_host" ] ; then
-    echo ERROR: status timed out
-    exit 1
-fi
-
-datadir=`mktemp -d ${TMPDIR:-/tmp}/wget.test.XXXXXXX`
-echo INFO: datadir=$datadir
-
-#slave_connected=$(
-#    wget --tries=100 --timeout=6 http://$slave_host:9000 -O - \
-#     -a $datadir/wget.log | grep -q 'UP!')
-
-assert_is_listening() {
-    local port=$1
-    listening=""
-    for try in `seq 1 10` ; do
-        if ! nc $slave_host $port < /dev/null ; then
-            continue
-        fi
-        listening="yes"
-        break
-    done
-
-    if [ -z "$listening" ] ; then
-       echo "FAIL: not listening on port $port after 10 retries"
-       return 1
-    else
-       echo "PASS: listening on port $port"
-       return 0
-    fi
-}
-
-assert_is_listening 9000
-
-touch $datadir/passed
-
-trap - EXIT
-teardown
-
-echo INFO: PASS
-exit 0

=== removed file 'tests/200_buildbot-slave.test'
--- tests/200_buildbot-slave.test	2012-02-07 12:33:51 +0000
+++ tests/200_buildbot-slave.test	1970-01-01 00:00:00 +0000
@@ -1,58 +0,0 @@
-#!/bin/sh
-
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-set -e
-
-teardown() {
-    juju destroy-service buildbot-slave
-    if [ -n "$datadir" ] ; then
-        if [ -f $datadir/passed ]; then
-            rm -r $datadir
-        else
-            echo $datadir preserved
-        fi
-    fi
-}
-trap teardown EXIT
-
-
-juju deploy buildbot-master
-juju deploy buildbot-slave
-juju add-relation buildbot-slave buildbot-master
-juju expose buildbot-master
-
-for try in `seq 1 600` ; do
-    master_host=`juju status | tests/get-unit-info buildbot-master public-address`
-    if [ -z "$master_host" ] ; then
-        sleep 1
-    else
-        break
-    fi
-done
-
-if [ -z "$master_host" ] ; then
-    echo ERROR: status timed out
-    exit 1
-fi
-
-datadir=`mktemp -d ${TMPDIR:-/tmp}/wget.test.XXXXXXX`
-echo INFO: datadir=$datadir
-
-slave_connected=$(
-    wget --tries=100 --timeout=6 http://$master_host:8010/buildslaves -O - \
-     -a $datadir/wget.log | grep -q 'Idle')
-
-if [ -z $slave_connected ]; then
-    echo "ERROR: The slave is not connected after 600 seconds."
-    exit 1
-fi
-
-touch $datadir/passed
-
-trap - EXIT
-teardown
-
-echo INFO: PASS
-exit 0

=== added file 'tests/buildbot-slave.test'
--- tests/buildbot-slave.test	1970-01-01 00:00:00 +0000
+++ tests/buildbot-slave.test	2012-02-08 18:09:23 +0000
@@ -0,0 +1,121 @@
+#!/usr/bin/python
+
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import os
+import time
+import unittest
+import urllib2
+
+from helpers import (
+    command,
+    encode_file,
+    unit_info,
+    )
+from openport import (
+    PORT,
+    TEXT,
+    )
+
+
+# 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')
+
+
+class TestCharm(unittest.TestCase):
+
+    def setUp(self):
+        self.charm_name = 'buildbot-slave'
+        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
+        args = ['deploy']
+        if self.repository is not None:
+            args.append('--repository=' + self.repository)
+            deploy_name = 'local:' + deploy_name
+        if self.environment is not None:
+            args.append('--environment=' + self.environment)
+        if config:
+            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.")
+
+    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)
+
+    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'
+        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:
+                relation_info = unit_info(self.charm_name, 'relations')
+                if relation_info['buildbot']['state'] == 'up':
+                    break
+                time.sleep(0.1)
+            try:
+                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)
+
+if __name__ == '__main__':
+    unittest.main()

=== modified file 'tests/config.test.yaml'
--- tests/config.test.yaml	2012-02-01 17:13:41 +0000
+++ tests/config.test.yaml	2012-02-08 18:09:23 +0000
@@ -1,6 +1,5 @@
 buildbot-slave:
-  installdir: /tmp/buildbot
-  name: example-slave
-  passwd: pass
-  script-retrieval-method: bzr
-  script-url: "http://bazaar.launchpad.net/~gmb/charms/oneiric/buildbot-slave/look-we-have-tests/tests/open-port.py"
+  script-retrieval-method: bzr_cat
+  script-url: "http://bazaar.launchpad.net/~yellow/charms/oneiric/buildbot-slave/charm-tests/tests/openport.py"
+  script-path: openport.py
+  script-args: -d

=== removed file 'tests/get-unit-info'
--- tests/get-unit-info	2012-02-07 12:33:51 +0000
+++ tests/get-unit-info	1970-01-01 00:00:00 +0000
@@ -1,50 +0,0 @@
-#!/usr/bin/python
-
-# Copyright 2012 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# machines:
-#   0: {dns-name: ec2-50-17-84-127.compute-1.amazonaws.com, instance-id: i-8c5c3fec}
-#   1: {dns-name: ec2-184-73-102-113.compute-1.amazonaws.com, instance-id: i-14a2c174}
-#   2: {dns-name: ec2-75-101-184-93.compute-1.amazonaws.com, instance-id: i-e0a2c180}
-# services:
-#   mysql:
-#     charm: local:mysql-11
-#     relations: {db: wordpress}
-#     units:
-#       mysql/0:
-#         machine: 2
-#         relations:
-#           db: {state: up}
-#         state: started
-#   wordpress:
-#     charm: local:wordpress-31
-#     exposed: true
-#     relations: {db: mysql}
-#     units:
-#       wordpress/0:
-#         machine: 1
-#         open-ports: []
-#         relations: {}
-#         state: null
-
-import yaml
-import sys
-from subprocess import Popen, PIPE
-
-
-def main():
-    d = yaml.safe_load(Popen(['juju','status'],stdout=PIPE).stdout)
-    srv = d.get("services", {}).get(sys.argv[1])
-    if srv is None:
-        return
-    units = srv.get("units", {})
-    if units is None:
-        return
-    item = units.items()[0][1].get(sys.argv[2])
-    if item is None:
-        return
-    print item
-
-if __name__ == "__main__":
-    main()

=== added symlink 'tests/helpers.py'
=== target is u'../../buildbot-master/hooks/helpers.py'
=== renamed file 'tests/open-port.py' => 'tests/openport.py'
--- tests/open-port.py	2012-02-07 12:33:51 +0000
+++ tests/openport.py	2012-02-08 18:09:23 +0000
@@ -6,26 +6,33 @@
 # A little python script to open a port on the buildslave so that we can
 # test that it's actually deployed successfully.
 
+import os
+import sys
 import subprocess
-import SimpleHTTPServer
-import SocketServer
+from BaseHTTPServer import (
+    BaseHTTPRequestHandler,
+    HTTPServer,
+    )
+
 
 PORT = 9000
-
-simple_html = """
-<html>
-    <head>
-        <title>Here's some HTML</title>
-    </head>
-    <body><p>The slave is UP!</p></body>
-</html>
-"""
-
-with open('/tmp/index.html', 'w') as index_file:
-    index_file.write(simple_html)
-
-subprocess.call(['open-port', '9000/TCP'])
-Handler = SimpleHTTPServer.SimpleHTTPRequestHandler
-httpd = SocketServer.TCPServer(("", PORT), Handler)
-#httpd.serve_forever()
-print "HERE I AM!!!"
+TEXT = 'The slave is UP!'
+
+
+class Handler(BaseHTTPRequestHandler):
+    def do_GET(self):
+        self.send_response(200, 'OK')
+        self.send_header('Content-type', 'text/palin')
+        self.end_headers()
+        self.wfile.write(TEXT)
+
+
+if __name__ == "__main__":
+    ## HACKETTY HACK.
+    options = sys.argv[1:]
+    if options and options[0] == '-d':
+        # OH MY GOD.
+        os.system('{} &'.format(sys.argv[0]))
+        sys.exit(0)
+    subprocess.call(['open-port', '{}/TCP'.format(PORT)])
+    HTTPServer(('', PORT), Handler).serve_forever()