Review: Approve code
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.
--
https://code.launchpad.net/~gmb/charms/oneiric/buildbot-slave/charm-tests/+merge/92098
Your team Launchpad Yellow Squad is subscribed to branch
lp:~yellow/charms/oneiric/buildbot-slave/trunk.
--
Mailing list: https://launchpad.net/~yellow
Post to : [email protected]
Unsubscribe : https://launchpad.net/~yellow
More help : https://help.launchpad.net/ListHelp
