yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #00388
Re: [Merge] lp:~gmb/charms/oneiric/buildbot-slave/charm-tests into lp:~yellow/charms/oneiric/buildbot-slave/trunk
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.
References