launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/test-maas-separate-media-for-dev into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/test-maas-separate-media-for-dev/+merge/96938

I was getting test errors because the tmp folder existed; the test was
blowing up when trying to create it too. But I didn't want to blow it
away because I was using the contents of it to experiment with odev.
Hence this branch gives the maas.demo and maas.development
configurations separate MEDIA_ROOTs.

See the contents of the new file media/README:

> This folder contains somewhat ephemeral things: subfolders serve as
> MEDIA_ROOT for maas.demo and maas.development environments. The
> media/demo directory should always exist and not be deleted, though
> its contents can be. The media/development directory should be
> created and destroyed by tests, as needed.

This is largely because it doesn't seem possible to change the value
of MEDIA_ROOT during a test run and get Django and everything else to
notice. Fwiw, I did try.

-- 
https://code.launchpad.net/~allenap/maas/test-maas-separate-media-for-dev/+merge/96938
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/test-maas-separate-media-for-dev into lp:maas.

=== modified file '.bzrignore'
--- .bzrignore	2012-02-20 11:02:48 +0000
+++ .bzrignore	2012-03-11 21:22:17 +0000
@@ -13,6 +13,8 @@
 ./download-cache
 ./eggs
 ./logs
+./media/demo/*
+./media/development
 ./parts
 ./pserv.log
 ./pserv.pid

=== modified file 'Makefile'
--- Makefile	2012-02-15 17:13:37 +0000
+++ Makefile	2012-03-11 21:22:17 +0000
@@ -66,6 +66,7 @@
 clean:
 	find . -type f -name '*.py[co]' -print0 | xargs -r0 $(RM)
 	find . -type f -name '*~' -print0 | xargs -r0 $(RM)
+	$(RM) -r media/demo/* media/development
 
 distclean: clean pserv-stop
 	utilities/maasdb delete-cluster ./db/

=== added directory 'media'
=== added file 'media/README'
--- media/README	1970-01-01 00:00:00 +0000
+++ media/README	2012-03-11 21:22:17 +0000
@@ -0,0 +1,5 @@
+This folder contains somewhat ephemeral things: subfolders serve as
+MEDIA_ROOT for maas.demo and maas.development environments. The
+media/demo directory should always exist and not be deleted, though
+its contents can be. The media/development directory should be created
+and destroyed by tests, as needed.

=== added directory 'media/demo'
=== modified file 'src/maas/demo.py'
--- src/maas/demo.py	2012-03-05 15:13:25 +0000
+++ src/maas/demo.py	2012-03-11 21:22:17 +0000
@@ -10,10 +10,14 @@
 
 __metaclass__ = type
 
+import os
+
 # SKIP, developement settings should override base settings.
 from maas.settings import *
 from maas.development import *
 
+MEDIA_ROOT = os.path.join(os.getcwd(), "media/demo")
+
 MIDDLEWARE_CLASSES += (
     'maasserver.middleware.ConsoleExceptionMiddleware',
     'debug_toolbar.middleware.DebugToolbarMiddleware',

=== modified file 'src/maas/development.py'
--- src/maas/development.py	2012-03-08 15:21:42 +0000
+++ src/maas/development.py	2012-03-11 21:22:17 +0000
@@ -39,7 +39,7 @@
 
 # Absolute filesystem path to the directory that will hold user-uploaded files.
 # Example: "/home/media/media.lawrence.com/media/"
-MEDIA_ROOT = os.path.join(os.getcwd(), "tmp")
+MEDIA_ROOT = os.path.join(os.getcwd(), "media/development")
 
 INSTALLED_APPS += (
     'django.contrib.admin',

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-03-06 08:22:51 +0000
+++ src/maasserver/tests/test_api.py	2012-03-11 21:22:17 +0000
@@ -777,10 +777,12 @@
 
     def setUp(self):
         super(FileStorageAPITest, self).setUp()
-        os.mkdir(settings.MEDIA_ROOT)
-        self.tmpdir = os.path.join(settings.MEDIA_ROOT, "testing")
+        media_root = settings.MEDIA_ROOT
+        self.assertFalse(os.path.exists(media_root), "See media/README")
+        self.addCleanup(shutil.rmtree, media_root, ignore_errors=True)
+        os.mkdir(media_root)
+        self.tmpdir = os.path.join(media_root, "testing")
         os.mkdir(self.tmpdir)
-        self.addCleanup(shutil.rmtree, settings.MEDIA_ROOT)
 
     def make_file(self, name="foo", contents="test file contents"):
         """Make a temp file named `name` with contents `contents`.

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-03-06 08:48:58 +0000
+++ src/maasserver/tests/test_models.py	2012-03-11 21:22:17 +0000
@@ -450,8 +450,6 @@
 class FileStorageTest(TestCase):
     """Testing of the :class:`FileStorage` model."""
 
-    FILEPATH = settings.MEDIA_ROOT
-
     def make_upload_dir(self):
         """Create the upload directory, and arrange for eventual deletion.
 
@@ -462,13 +460,12 @@
         :return: Absolute path to the `FileStorage` upload directory.  This
             is the directory where the actual files are stored.
         """
-        # These will blow up if either directory already exists.  Which
-        # is brittle, but probably for the best since we ruthlessly
-        # delete them on cleanup!
-        os.mkdir(self.FILEPATH)
-        upload_dir = os.path.join(self.FILEPATH, FileStorage.upload_dir)
+        media_root = settings.MEDIA_ROOT
+        self.assertFalse(os.path.exists(media_root), "See media/README")
+        self.addCleanup(shutil.rmtree, media_root, ignore_errors=True)
+        os.mkdir(media_root)
+        upload_dir = os.path.join(media_root, FileStorage.upload_dir)
         os.mkdir(upload_dir)
-        self.addCleanup(shutil.rmtree, self.FILEPATH)
         return upload_dir
 
     def get_media_path(self, filename):