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

Proposed by Benji York on 2012-02-09
Status: Merged
Approved by: Brad Crittenden on 2012-02-10
Approved revision: 23
Merged at revision: 23
Proposed branch: lp:~benji/charms/oneiric/buildbot-master/tests-with-config-up-front
Merge into: lp:~yellow/charms/oneiric/buildbot-master/trunk
Diff against target: 89 lines (+51/-14)
1 file modified
tests/buildbot-master.test (+51/-14)
To merge this branch: bzr merge lp:~benji/charms/oneiric/buildbot-master/tests-with-config-up-front
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code 2012-02-09 Approve on 2012-02-10
Review via email: mp+92377@code.launchpad.net

Description of the change

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

To post a comment you must log in.
24. By Benji York on 2012-02-10

add a comment about temp file deletion

Brad Crittenden (bac) wrote :

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) wrote :

> 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/buildbot-master.test'
2--- tests/buildbot-master.test 2012-02-08 22:01:59 +0000
3+++ tests/buildbot-master.test 2012-02-10 13:13:18 +0000
4@@ -7,15 +7,35 @@
5 encode_file,
6 run,
7 unit_info,
8+ wait_for_page_contents,
9 wait_for_unit,
10 )
11-import base64
12 import os.path
13+import tempfile
14 import unittest
15-import urllib2
16+import yaml
17
18 juju = command('juju')
19
20+def make_charm_config_file(charm_config):
21+ charm_config_file = tempfile.NamedTemporaryFile()
22+ charm_config_file.write(yaml.dump(charm_config))
23+ charm_config_file.flush()
24+ # The NamedTemporaryFile instance is returned instead of just the name
25+ # because we want to take advantage of garbage collection-triggered
26+ # deletion of the temp file when it goes out of scope in the caller.
27+ return charm_config_file
28+
29+
30+def deploy(charm_config):
31+ charm_config_file = make_charm_config_file(charm_config)
32+ juju('deploy', 'buildbot-master', '--config='+charm_config_file.name)
33+ wait_for_unit('buildbot-master')
34+ addr = unit_info('buildbot-master', 'public-address')
35+ url = 'http://{}:8010'.format(addr)
36+ wait_for_page_contents(url, 'Welcome to the Buildbot')
37+
38+
39 class TestCharm(unittest.TestCase):
40
41 def tearDown(self):
42@@ -30,18 +50,35 @@
43 self.assertEqual(unit_info('buildbot-master', 'state'), 'started')
44
45 def test_port_opened(self):
46- juju('deploy', 'buildbot-master')
47- juju('set', 'buildbot-master', 'extra-packages=git')
48- config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
49- config = encode_file(config_path)
50- juju('set', 'buildbot-master', 'config-file='+config)
51- wait_for_unit('buildbot-master')
52- addr = unit_info('buildbot-master', 'public-address')
53- try:
54- page = urllib2.urlopen('http://{}:8010'.format(addr)).read()
55- except urllib2.URLError:
56- self.fail('could not fetch buildbot master status page')
57- self.assertIn('Welcome to the Buildbot', page)
58+ # Deploying a buildbot master should result in it opening a port and
59+ # serving its status via HTTP.
60+ bb_config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
61+ charm_config = {
62+ 'buildbot-master': {
63+ 'extra-packages': 'git',
64+ 'installdir': '/tmp/buildbot',
65+ 'config-file': encode_file(bb_config_path),
66+ }}
67+ deploy(charm_config)
68+
69+ def test_lpbuildbot(self):
70+ # Deploying a Launchpad-specific buildbot master does a good job of
71+ # exercising the configuration parameters. For example, the
72+ # configuration in this test adds a repositroy (lucid main universe),
73+ # installs a non-default buildbot package, and fetches the buildbot
74+ # configuration from bzr.
75+ charm_config = {
76+ 'buildbot-master': {
77+ 'buildbot-pkg': 'buildbot/lucid',
78+ 'config-transport': 'bzr',
79+ 'config-url':
80+ 'http://bazaar.launchpad.net/~launchpad/lpbuildbot/public',
81+ 'extra-repository':
82+ 'deb http://us.archive.ubuntu.com/ubuntu/'
83+ ' lucid main universe',
84+ 'installdir': '/var/lib/buildbot/masters/lpbuildbot',
85+ }}
86+ deploy(charm_config)
87
88
89 if __name__ == '__main__':

Subscribers

People subscribed via source and target branches

to all changes: