Bug#754143: Fix the tests to not pollute the data directory
Hi Raphaël,
Raphael Hertzog wrote:
> On Wed, 09 Jul 2014, Christophe Siraut wrote:
> > > I'm not sure what's the best approach but I see two clean solutions:
> > >
> > > - something global implemented as a derivative class of TestCase that
> > > does the required directory creation and settings change in self.setUp
> > > and drops the directory with a function recorded with self.addCleanup
> >
> > See attached patch. Feel free to adapt, I am unsure where to put the
> > subclass bits.
>
> Thanks, it's a good base but here are more things that I'd like to see:
>
> 1/ we move this to distro_tracker.test and and we provide replacements for
> all of SimpleTestCase, TestCase and LiveServerTestCase
> (we should use a mixin class to factorize the code between
> all those)
> ALTERNATIVE: we don't do all the classe but just the mixin
> and we modify only the test cases that need it to insert
> the mixin there...
>
> 2/ we override all the settings which are derived from
> DISTRO_TRACKER_DATA_PATH (even directories which are only
> used to read data should point to empty directories and not
> to directories with possible production data)
>
> 3/ ideally we should also restore the settings on tearDown so that we
> don't interfer with tests that are in standard unittest.TestCase...
>
> 4/ we add unit tests for the new classes (except maybe for
> LiveServerTestCase which might be a bit heavy...) to make
> sure that the settings point to empty directories which
> are outside of DISTRO_TRACKER_BASE_PATH
>
> 5/ some docstring documentation of the new classes would be welcome
>
> Once this is done we obviously have some cleanups to do:
>
> * change all our tests to use those classes
> * get rid of core.tests.common.temporary_media_dir() since we handle that
> at the class level now
> * merge rest of distro_tracker.core.tests.common in distro_tracker.test
The following patch should solve the issues you mention. Only the
cleanup part remains, I am waiting for your review before doing that.
Note we override the __call__ method instead of setUp, because the
latter gets overriden by the tests instances.
Best regards,
Christophe
---
distro_tracker/test/__init__.py | 58 +++++++++++++++++++++++++++++++++
distro_tracker/test/tests.py | 34 +++++++++++++++++++
distro_tracker/vendor/debian/tests.py | 3 +-
3 files changed, 94 insertions(+), 1 deletion(-)
create mode 100644 distro_tracker/test/__init__.py
create mode 100644 distro_tracker/test/tests.py
diff --git a/distro_tracker/test/__init__.py b/distro_tracker/test/__init__.py
new file mode 100644
index 0000000..61c5923
--- /dev/null
+++ b/distro_tracker/test/__init__.py
@@ -0,0 +1,58 @@
+# -*- coding: utf-8 -*-
+
+# Copyright 2013 The Distro Tracker Developers
+# See the COPYRIGHT file at the top-level directory of this distribution and
+# at http://deb.li/DTAuthors
+#
+# This file is part of Distro Tracker. It is subject to the license terms
+# in the LICENSE file found in the top-level directory of this
+# distribution and at http://deb.li/DTLicense. No part of Distro Tracker,
+# including this file, may be copied, modified, propagated, or distributed
+# except according to the terms contained in the LICENSE file.
+
+"""
+Distro Tracker test module.
+"""
+
+from django.test import TestCase as DjangoTestCase
+from django.test import SimpleTestCase as DjangoSimpleTestCase
+from django.test import LiveServerTestCase as DjangoLiveServerTestCase
+from django.conf import settings
+import shutil
+import tempfile
+import copy
+
+class CleanMixin(object):
+ """
+ Use temporary folders while testing
+ """
+ folders = ('STATIC_ROOT',
+ 'MEDIA_ROOT',
+ 'DISTRO_TRACKER_CACHE_DIRECTORY',
+ 'DISTRO_TRACKER_KEYRING_DIRECTORY',
+ 'DISTRO_TRACKER_TEMPLATE_DIRECTORY',
+ 'DISTRO_TRACKER_LOG_DIRECTORY')
+ settings_copy = copy.deepcopy(settings)
+
+ def __call__(self, result=None):
+ for f in self.folders:
+ d = tempfile.mkdtemp()
+ setattr(settings, f, d)
+ self.addCleanup(shutil.rmtree, d)
+ super(CleanMixin, self).__call__(result=result)
+
+ def tearDown(self):
+ for f in self.folders:
+ settings = self.settings_copy
+
+
+class TestCase(CleanMixin, DjangoTestCase):
+ pass
+
+
+class SimpleTestCase(CleanMixin, DjangoSimpleTestCase):
+ pass
+
+
+class LiveServerTestCase(CleanMixin, DjangoLiveServerTestCase):
+ pass
diff --git a/distro_tracker/test/tests.py b/distro_tracker/test/tests.py
new file mode 100644
index 0000000..977ae7c
--- /dev/null
+++ b/distro_tracker/test/tests.py
@@ -0,0 +1,34 @@
+# -*- coding: utf-8 -*-
+
+# Copyright 2013 The Distro Tracker Developers
+# See the COPYRIGHT file at the top-level directory of this distribution and
+# at http://deb.li/DTAuthors
+#
+# This file is part of Distro Tracker. It is subject to the license terms
+# in the LICENSE file found in the top-level directory of this
+# distribution and at http://deb.li/DTLicense. No part of Distro Tracker,
+# including this file, may be copied, modified, propagated, or distributed
+# except according to the terms contained in the LICENSE file.
+
+"""
+Tests for test functionalities of Distro Tracker.
+"""
+
+from __future__ import unicode_literals
+from distro_tracker.test import TestCase
+from distro_tracker.test import CleanMixin
+from django.conf import settings
+import copy
+
+settings_copy = copy.deepcopy(settings)
+
+
+class SetTemporaryFolders(TestCase):
+
+ def test_set_temporary_folders(self):
+ """
+ Tests the use of temporary folders
+ """
+ for f in CleanMixin().folders:
+ self.assertNotEqual(getattr(settings, f),
+ getattr(settings_copy, f))
diff --git a/distro_tracker/vendor/debian/tests.py b/distro_tracker/vendor/debian/tests.py
index 0848044..851cf62 100644
--- a/distro_tracker/vendor/debian/tests.py
+++ b/distro_tracker/vendor/debian/tests.py
@@ -15,7 +15,8 @@ Tests for Debian-specific modules/functionality of Distro Tracker.
"""
from __future__ import unicode_literals
-from django.test import TestCase, SimpleTestCase
+from django.test import SimpleTestCase
+from distro_tracker.test import TestCase
from django.test.utils import override_settings
from django.core import mail
from django.core.urlresolvers import reverse
--
1.7.10.4
Reply to: