Merge lp:~benji/charms/oneiric/buildbot-master/tests-with-config-up-front into lp:~yellow/charms/oneiric/buildbot-master/trunk

[Benji York]

This branch adds tests for the configure-things-up-front scenario.

All the configuration options for the buildbot-master should be covered
now except for config-user and config-private-key, both of which I
/think/ can go away.

Tests pass:

    % python hooks/tests.py
    ..............
    ----------------------------------------------------------------------
    Ran 14 tests in 0.047s

    % RESOLVE_TEST_CHARMS=1 tests/buildbot-master.test; beep
    ...
    ----------------------------------------------------------------------
    Ran 3 tests in 119.182s

[lp:~benji/charms/oneiric/buildbot-master/tests-with-config-up-front]

24. By Benji York on 2012-02-10

add a comment about temp file deletion

[Brad Crittenden (bac)]

Firstly, let's not get rid of the user/key stuff just yet. It may need to be repurposed for the "persist history before dying" effort.

As we discussed on IRC please comment why you use flush() on the temporary file so someone doesn't inadvertantly "fix" it.

We also discussed how our test method encode_file doesn't work in the manner we'd use in real life so it may be better to rewrite it to use the UNIX 'base64' command. We used to suggest 'uuencode' which didn't work with our decoding. So I do think we should test the method we suggest people use.

Otherwise a nice addition to the tests.

review: Approve (code)

[Benji York (benji)]

> As we discussed on IRC please comment why you use flush() on the temporary
> file so someone doesn't inadvertantly "fix" it.

I added a comment explaining the apparent oddity.

> We also discussed how our test method encode_file doesn't work in the manner
> we'd use in real life so it may be better to rewrite it to use the UNIX
> 'base64' command. We used to suggest 'uuencode' which didn't work with our
> decoding. So I do think we should test the method we suggest people use.

Yep, we should have a test like that. If we don't remove the need for base64
encoding alltogether, I'll add one in another branch.

=== modified file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	2012-02-08 22:01:59 +0000
+++ tests/buildbot-master.test	2012-02-10 13:13:18 +0000
@@ -7,15 +7,35 @@
     encode_file,
     run,
     unit_info,
+    wait_for_page_contents,
     wait_for_unit,
     )
-import base64
 import os.path
+import tempfile
 import unittest
-import urllib2
+import yaml
 
 juju = command('juju')
 
+def make_charm_config_file(charm_config):
+    charm_config_file = tempfile.NamedTemporaryFile()
+    charm_config_file.write(yaml.dump(charm_config))
+    charm_config_file.flush()
+    # The NamedTemporaryFile instance is returned instead of just the name
+    # because we want to take advantage of garbage collection-triggered
+    # deletion of the temp file when it goes out of scope in the caller.
+    return charm_config_file
+
+
+def deploy(charm_config):
+    charm_config_file = make_charm_config_file(charm_config)
+    juju('deploy', 'buildbot-master', '--config='+charm_config_file.name)
+    wait_for_unit('buildbot-master')
+    addr = unit_info('buildbot-master', 'public-address')
+    url = 'http://{}:8010'.format(addr)
+    wait_for_page_contents(url, 'Welcome to the Buildbot')
+
+
 class TestCharm(unittest.TestCase):
 
     def tearDown(self):
@@ -30,18 +50,35 @@
         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:
-            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)
+        # Deploying a buildbot master should result in it opening a port and
+        # serving its status via HTTP.
+        bb_config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
+        charm_config = {
+            'buildbot-master': {
+                'extra-packages': 'git',
+                'installdir': '/tmp/buildbot',
+                'config-file': encode_file(bb_config_path),
+            }}
+        deploy(charm_config)
+
+    def test_lpbuildbot(self):
+        # Deploying a Launchpad-specific buildbot master does a good job of
+        # exercising the configuration parameters.  For example, the
+        # configuration in this test adds a repositroy (lucid main universe),
+        # installs a non-default buildbot package, and fetches the buildbot
+        # configuration from bzr.
+        charm_config = {
+            'buildbot-master': {
+                'buildbot-pkg': 'buildbot/lucid',
+                'config-transport': 'bzr',
+                'config-url':
+                    'http://bazaar.launchpad.net/~launchpad/lpbuildbot/public',
+                'extra-repository':
+                    'deb http://us.archive.ubuntu.com/ubuntu/'
+                    ' lucid main universe',
+                'installdir': '/var/lib/buildbot/masters/lpbuildbot',
+            }}
+        deploy(charm_config)
 
 
 if __name__ == '__main__':