Merge lp:~jtv/maas/db-fixture into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Start test database cluster from test runner.

Set up the testing database cluster as part of the test runner.

It took me a while to find a hook into the test runner that I could use. The one I found was: use a custom test runner based on the django one, but with a call to maasdb added to it. I couldn't define the custom runner in development.py or maas/__init__.py etc. because it'd just lead to circular imports and a mysterious failure.

To try this out, create a new branch (so there's certainly no database cluster running in it), "make," then check "ps -ef | grep postgres" to verify that there still is no postgres running in the branch, then "./bin/test" to see that the tests can access the database. I added some only-slightly-more-meaningful tests to make sure of that.

You may notice that database setup before a test takes a while. That doesn't go away when I remove the custom test runner though; I guess it's something we just can't get rid of.

Something a bit nasty I did was to hard-code "./db/" as the database cluster location. Ideally I'd have some way of specifying "main branch directory/db."

The maasdb script does nothing when asked to start a cluster that's already running. Doesn't take very long to find out, either.

Jeroen

[Gavin Panella (allenap)]

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.

review: Approve

[Gavin Panella (allenap)]

Fwiw, I'd still prefer to use van.pg, but this works and that's plenty for now :)

[Jeroen T. Vermeulen (jtv)]

> + 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/'])

Nice, thanks.

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

Done. I changed 2 things:

1. Pass stdout=PIPE to check_call, which eats the routine output when running implicitly from bin/test.

2. Initialize the cluster with "-A trust" to suppress the warning about defaulting to trust authentication.

Together, those completely silence successful cluster initialization from bin/test.

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

Thanks for pointing that out. I'll remove this whole test file then.

Jeroen

[Jeroen T. Vermeulen (jtv)]

As for van.pg, it's attractive to have this job done by code from others but several things made me decide against it:

 * As far as I can tell, it's not packaged for Ubuntu. Dealing with that could be more trouble than rolling our own.

 * It depends on testresources, which is in universe.

 * The documentation didn't tell me what I needed to know, so I got lazy and just wrote my own.

 * Both django and van.pg try to create the test databases themselves, so there's a bit of overlap.

 * We have more flexibility to tweak our own, purpose-built code. For instance, our code can create a non-disposable database if necessary.

[Gavin Panella (allenap)]

> 1. Pass stdout=PIPE to check_call, which eats the routine output when
> running implicitly from bin/test.

This can deadlock when the pipe's buffer fills up. Instead stdout
could be redirected to a temporary file, or just to /dev/null:

  with open(os.devnull, "wb") as devnull:
      check_call([...], stdout=devnull)

> 2. Initialize the cluster with "-A trust" to suppress the warning
> about defaulting to trust authentication.

Cool :)

=== modified file 'Makefile'
--- Makefile	2012-01-17 15:04:53 +0000
+++ Makefile	2012-01-17 19:10:32 +0000
@@ -11,7 +11,7 @@
 dev-db:
 	bin/maasdb start ./db/ disposable
 
-test: bin/test dev-db
+test: bin/test
 	bin/test
 
 lint:

=== modified file 'README.txt'
--- README.txt	2012-01-17 09:58:26 +0000
+++ README.txt	2012-01-17 19:10:32 +0000
@@ -8,9 +8,9 @@
 
 Access to the database is configured in src/maas/development.py.
 
-The Makefile sets up a development database cluster inside your branch.  It
-lives in the "db" directory, which gets created on demand.  You'll want to
-shut it down before deleting a branch; see below.
+The Makefile or the test suite sets up a development database cluster inside
+your branch.  It lives in the "db" directory, which gets created on demand.
+You'll want to shut it down before deleting a branch; see below.
 
 First, set up the project.  This fetches all the required dependencies, and
 creates a local database cluster and development database:

=== modified file 'bin/maasdb'
--- bin/maasdb	2012-01-17 10:21:54 +0000
+++ bin/maasdb	2012-01-17 19:10:32 +0000
@@ -1,6 +1,6 @@
 #! /bin/bash -e
 #
-# MaaS database control script
+# MaaS database control script.  See main() at the bottom for usage.
 #
 # Most functions take as their first argument a database cluster's data
 # directory.  This is where the database's socket, pidfile, log, and data will
@@ -44,7 +44,7 @@
     if ! test -d "$DATADIR/base"
     then
         mkdir -p -- "$DATADIR"
-        $PGCTL init -D "$DATADIR" -o '-E utf8'
+        $PGCTL init -D "$DATADIR" -o '-E utf8 -A trust'
     fi
 }
 

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-01-16 17:02:09 +0000
+++ src/maas/development.py	2012-01-17 19:10:32 +0000
@@ -5,6 +5,10 @@
 
 import os
 
+# Use our custom test runner, which makes sure that a local database
+# cluster is running in the branch.
+TEST_RUNNER='maas.testing.runner.CustomTestRunner'
+
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
 YUI_DEBUG = DEBUG

=== added directory 'src/maas/testing'
=== added file 'src/maas/testing/__init__.py'
=== added file 'src/maas/testing/runner.py'
--- src/maas/testing/runner.py	1970-01-01 00:00:00 +0000
+++ src/maas/testing/runner.py	2012-01-17 19:10:32 +0000
@@ -0,0 +1,12 @@
+from subprocess import check_call, PIPE
+from django.test.simple import DjangoTestSuiteRunner
+
+
+class CustomTestRunner(DjangoTestSuiteRunner):
+    """Custom test runner; ensures that the test database cluster is up."""
+
+    def setup_databases(self, *args, **kwargs):
+        """Fire up the db cluster, then punt to original implementation."""
+        check_call(
+            ['bin/maasdb', 'start', './db/', 'disposable'], stdout=PIPE)
+        return super(CustomTestRunner, self).setup_databases(*args, **kwargs)

=== removed file 'src/maasserver/tests/simple_tests.py'
--- src/maasserver/tests/simple_tests.py	2012-01-17 15:35:41 +0000
+++ src/maasserver/tests/simple_tests.py	1970-01-01 00:00:00 +0000
@@ -1,16 +0,0 @@
-"""
-This file demonstrates writing tests using the unittest module. These will pass
-when you run "manage.py test".
-
-Replace this with more appropriate tests for your application.
-"""
-
-from django.test import TestCase
-
-
-class SimpleTest(TestCase):
-    def test_basic_addition(self):
-        """
-        Tests that 1 + 1 always equals 2.
-        """
-        self.assertEqual(1 + 1, 2)