Merge lp://staging/~fwereade/pyjuju/add-upstart-class into lp://staging/pyjuju

+1. Just minors here:

[1]

+description "blah"
+author "Juju Team <email address hidden>"
+
+start on runlevel [2345]
+stop on runlevel [!2345]
+respawn
+
+env BLAH="blah blah"
+
+exec /bin/imagination-failure --no-ideas

Having a generating family of dummy data (eg your Star Wars theme or really whatever)
can be helpful for seeing the expected patterns. "blah blah" is unfortunately
just too much noise when looking at these tests.

The other thing is that putting these files inline might work much better, to avoid one
more place to look to see what the expected output should be.

[2]

+class UpstartServiceTest(TestCase):
+
+ def setUp(self):
+ self.root = self.makeDir()
+ init_dir = os.path.join(self.root, "etc", "init")
+ os.makedirs(init_dir)
+ self.conf = os.path.join(init_dir, "some-name.conf")
+ self.patch(UpstartService, "root_path", self.root)
+ self.service = UpstartService("some-name")

It's unlikely this will be mixed in with other tests, but it's likely best to preserve
the existing cooperative super inheritance support, including returning a Deferred
(usually by inlineCallbacks).

[3]

+ def test_is_installed(self):

Doc strings for tests!

+ self.assertFalse(self.service.is_installed())
+ self.write_dummy_conf()
+ self.assertTrue(self.service.is_installed())
+

[4]

+ @inlineCallbacks
+ def test_is_running(self):

Docs/comments especially here. Best to be explicit about how the mocks are
setting things up, especially when there's a sequence of multiple calls as seen here.

lgtm +1, some minors

[0]

    # on class for ease of mocking

s/mocking/testing

[1]

+ self.conf = os.path.join(init_dir, "some-name.conf")

pep8 cheese, double space after =

[2]

+ def set_start_on_filesystem(self):
+ self._start_on_filesystem = True

this line is never tested, nor entirely clear on usage. There are several upstart gurus in the room if this needs clarification.

[3] more untested lines

+ self.root_path = root_path

its not clear why this mocked out as a class attribute, when the constructor takes the arg.

+ raise ServiceError("Cannot render .conf: no description set")
+ raise ServiceError("Cannot render .conf: no command set")

[0,1]

Cool, thanks

[2,3]

...gaah, missing @inlineCallbacks~s led to some untestedness; root_path tested explicitly. After consultation with bcsaller (original author), it's fine to drop the "start on filesystem or" complication, so it's gone.