Code review comment for lp:~jtv/maas/db-fixture

This is a neat solution :)

[1]

+ def setup_databases(self, *args, **kwargs):
+ """Fire up the db cluster, then punt to original implementation."""
+ process = Popen(['bin/maasdb', 'start', './db/'])
+ retval = process.wait()
+ if retval != 0:
+ raise RuntimeError("Failed to start database cluster.")

Consider subprocess.check_call() here; it'll raise CalledProcessError
if the command does not exit with 0. Four lines into one:

        check_call(['bin/maasdb', 'start', './db/'])

It would also be nice to make this quiet unless there's an
error. Django doesn't seem to know about subunit output yet, but it
would be nice not to muddy things in advance. Witness how long it has
taken to make the Launchpad test suite STFU.

[2]

 class SimpleTest(TestCase):
- def test_basic_addition(self):
- """
- Tests that 1 + 1 always equals 2.
- """
- self.assertEqual(1 + 1, 2)
+
+ def test_can_create_nodes(self):
+ self.assertEqual([], list(Node.objects.all()))
+ n = Node(name='foo', status='NEW')
+ n.save()
+ self.assertEqual([n], list(Node.objects.all()))
+
+ def test_no_nodes_exist_initially(self):
+ self.assertEqual([], list(Node.objects.all()))
+

Raphael's recent changes provide enough to exercise your change, so
consider removing this.