diff mbox

[07/11] tests: Move 'reverse' calls inside 'setUp'

Message ID 1435654329-19960-8-git-send-email-stephen.finucane@intel.com
State Superseded
Headers show

Commit Message

Stephen Finucane June 30, 2015, 8:52 a.m. UTC
Django creates test databases after it loads tests. If operations that
access the database occur at class level, these will be executed before
the test database has been created. Fix this by moving these calls into
the relevant 'setUp' functions for each test.

In addition to these changes, remove some obviously dead imports.

Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
---
 patchwork/tests/test_confirm.py       |  1 -
 patchwork/tests/test_mail_settings.py | 41 ++++++++++++++++-------------------
 patchwork/tests/test_notifications.py |  4 +---
 patchwork/tests/test_user.py          | 13 ++++++-----
 4 files changed, 28 insertions(+), 31 deletions(-)

Comments

Damien Lespiau Aug. 19, 2015, 1:38 p.m. UTC | #1
On Tue, Jun 30, 2015 at 09:52:05AM +0100, Stephen Finucane wrote:
> Django creates test databases after it loads tests. If operations that
> access the database occur at class level, these will be executed before
> the test database has been created. Fix this by moving these calls into
> the relevant 'setUp' functions for each test.

You're describing what you change but forgot to tell why. Why do we need
that? the reverse function isn't going to hit the database is it?

> In addition to these changes, remove some obviously dead imports.

These sentences are usually a good hint it needed a separate patch :)

> Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
> ---
>  patchwork/tests/test_confirm.py       |  1 -
>  patchwork/tests/test_mail_settings.py | 41 ++++++++++++++++-------------------
>  patchwork/tests/test_notifications.py |  4 +---
>  patchwork/tests/test_user.py          | 13 ++++++-----
>  4 files changed, 28 insertions(+), 31 deletions(-)
> 
> diff --git a/patchwork/tests/test_confirm.py b/patchwork/tests/test_confirm.py
> index fad5125..9fe938e 100644
> --- a/patchwork/tests/test_confirm.py
> +++ b/patchwork/tests/test_confirm.py
> @@ -17,7 +17,6 @@
>  # along with Patchwork; if not, write to the Free Software
>  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  
> -import unittest
>  from django.test import TestCase
>  from django.contrib.auth.models import User
>  from django.core.urlresolvers import reverse
> diff --git a/patchwork/tests/test_mail_settings.py b/patchwork/tests/test_mail_settings.py
> index a193c97..477f39f 100644
> --- a/patchwork/tests/test_mail_settings.py
> +++ b/patchwork/tests/test_mail_settings.py
> @@ -17,19 +17,17 @@
>  # along with Patchwork; if not, write to the Free Software
>  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  
> -import unittest
>  import re
>  from django.test import TestCase
> -from django.test.client import Client
>  from django.core import mail
>  from django.core.urlresolvers import reverse
> -from django.contrib.auth.models import User
>  from patchwork.models import EmailOptout, EmailConfirmation, Person
>  from patchwork.tests.utils import create_user, error_strings
>  
>  class MailSettingsTest(TestCase):
> -    view = 'patchwork.views.mail.settings'
> -    url = reverse(view)
> +
> +    def setUp(self):
> +        self.url = reverse('patchwork.views.mail.settings')
>  
>      def testMailSettingsGET(self):
>          response = self.client.get(self.url)
> @@ -78,8 +76,9 @@ class MailSettingsTest(TestCase):
>          self.assertTrue(('action="%s"' % optin_url) in response.content)
>  
>  class OptoutRequestTest(TestCase):
> -    view = 'patchwork.views.mail.optout'
> -    url = reverse(view)
> +
> +    def setUp(self):
> +        self.url = reverse('patchwork.views.mail.optout')
>  
>      def testOptOutRequestGET(self):
>          response = self.client.get(self.url)
> @@ -124,10 +123,9 @@ class OptoutRequestTest(TestCase):
>          self.assertEquals(len(mail.outbox), 0)
>  
>  class OptoutTest(TestCase):
> -    view = 'patchwork.views.mail.optout'
> -    url = reverse(view)
>  
>      def setUp(self):
> +        self.url = reverse('patchwork.views.mail.optout')
>          self.email = u'foo@example.com'
>          self.conf = EmailConfirmation(type = 'optout', email = self.email)
>          self.conf.save()
> @@ -157,10 +155,9 @@ class OptoutPreexistingTest(OptoutTest):
>          EmailOptout(email = self.email).save()
>  
>  class OptinRequestTest(TestCase):
> -    view = 'patchwork.views.mail.optin'
> -    url = reverse(view)
>  
>      def setUp(self):
> +        self.url = reverse('patchwork.views.mail.optin')
>          self.email = u'foo@example.com'
>          EmailOptout(email = self.email).save()
>  
> @@ -232,8 +229,9 @@ class OptinTest(TestCase):
>  
>  class OptinWithoutOptoutTest(TestCase):
>      """Test an opt-in with no existing opt-out"""
> -    view = 'patchwork.views.mail.optin'
> -    url = reverse(view)
> +
> +    def setUp(self):
> +        self.url = reverse('patchwork.views.mail.optin')
>  
>      def testOptInWithoutOptout(self):
>          email = u'foo@example.com'
> @@ -248,16 +246,15 @@ class UserProfileOptoutFormTest(TestCase):
>      """Test that the correct optin/optout forms appear on the user profile
>         page, for logged-in users"""
>  
> -    view = 'patchwork.views.user.profile'
> -    url = reverse(view)
> -    optout_url = reverse('patchwork.views.mail.optout')
> -    optin_url = reverse('patchwork.views.mail.optin')
> -    form_re_template = ('<form\s+[^>]*action="%(url)s"[^>]*>'
> -                        '.*?<input\s+[^>]*value="%(email)s"[^>]*>.*?'
> -                        '</form>')
> -    secondary_email = 'test2@example.com'
> -
>      def setUp(self):
> +        self.url = reverse('patchwork.views.user.profile')
> +        self.optout_url = reverse('patchwork.views.mail.optout')
> +        self.optin_url = reverse('patchwork.views.mail.optin')
> +        self.form_re_template = ('<form\s+[^>]*action="%(url)s"[^>]*>'
> +                                 '.*?<input\s+[^>]*value="%(email)s"[^>]*>.*?'
> +                                 '</form>')
> +        self.secondary_email = 'test2@example.com'
> +
>          self.user = create_user()
>          self.client.login(username = self.user.username,
>                  password = self.user.username)
> diff --git a/patchwork/tests/test_notifications.py b/patchwork/tests/test_notifications.py
> index c46af61..37adb8d 100644
> --- a/patchwork/tests/test_notifications.py
> +++ b/patchwork/tests/test_notifications.py
> @@ -19,12 +19,10 @@
>  
>  import datetime
>  from django.test import TestCase
> -from django.core.urlresolvers import reverse
>  from django.core import mail
>  from django.conf import settings
> -from django.db.utils import IntegrityError
>  from patchwork.models import Patch, State, PatchChangeNotification, EmailOptout
> -from patchwork.tests.utils import defaults, create_maintainer
> +from patchwork.tests.utils import defaults
>  from patchwork.utils import send_notifications
>  
>  class PatchNotificationModelTest(TestCase):
> diff --git a/patchwork/tests/test_user.py b/patchwork/tests/test_user.py
> index 0faa970..d9aef7e 100644
> --- a/patchwork/tests/test_user.py
> +++ b/patchwork/tests/test_user.py
> @@ -17,9 +17,7 @@
>  # along with Patchwork; if not, write to the Free Software
>  # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  
> -import unittest
>  from django.test import TestCase
> -from django.test.client import Client
>  from django.core import mail
>  from django.core.urlresolvers import reverse
>  from django.conf import settings
> @@ -27,6 +25,7 @@ from django.contrib.auth.models import User
>  from patchwork.models import EmailConfirmation, Person, Bundle
>  from patchwork.tests.utils import defaults, error_strings
>  
> +
>  def _confirmation_url(conf):
>      return reverse('patchwork.views.confirm', kwargs = {'key': conf.key})
>  
> @@ -121,7 +120,7 @@ class UserPersonConfirmTest(TestCase):
>          self.assertEquals(conf.active, False)
>  
>  class UserLoginRedirectTest(TestCase):
> -    
> +
>      def testUserLoginRedirect(self):
>          url = '/user/'
>          response = self.client.get(url)
> @@ -157,8 +156,12 @@ class UserProfileTest(TestCase):
>          self.assertContains(response, bundle.get_absolute_url())
>  
>  class UserPasswordChangeTest(TestCase):
> -    form_url = reverse('django.contrib.auth.views.password_change')
> -    done_url = reverse('django.contrib.auth.views.password_change_done')
> +    user = None
> +
> +    def setUp(self):
> +        self.form_url = reverse('django.contrib.auth.views.password_change')
> +        self.done_url = reverse(
> +            'django.contrib.auth.views.password_change_done')
>  
>      def testPasswordChangeForm(self):
>          self.user = TestUser()
> -- 
> 2.0.0
> 
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
diff mbox

Patch

diff --git a/patchwork/tests/test_confirm.py b/patchwork/tests/test_confirm.py
index fad5125..9fe938e 100644
--- a/patchwork/tests/test_confirm.py
+++ b/patchwork/tests/test_confirm.py
@@ -17,7 +17,6 @@ 
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-import unittest
 from django.test import TestCase
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
diff --git a/patchwork/tests/test_mail_settings.py b/patchwork/tests/test_mail_settings.py
index a193c97..477f39f 100644
--- a/patchwork/tests/test_mail_settings.py
+++ b/patchwork/tests/test_mail_settings.py
@@ -17,19 +17,17 @@ 
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-import unittest
 import re
 from django.test import TestCase
-from django.test.client import Client
 from django.core import mail
 from django.core.urlresolvers import reverse
-from django.contrib.auth.models import User
 from patchwork.models import EmailOptout, EmailConfirmation, Person
 from patchwork.tests.utils import create_user, error_strings
 
 class MailSettingsTest(TestCase):
-    view = 'patchwork.views.mail.settings'
-    url = reverse(view)
+
+    def setUp(self):
+        self.url = reverse('patchwork.views.mail.settings')
 
     def testMailSettingsGET(self):
         response = self.client.get(self.url)
@@ -78,8 +76,9 @@  class MailSettingsTest(TestCase):
         self.assertTrue(('action="%s"' % optin_url) in response.content)
 
 class OptoutRequestTest(TestCase):
-    view = 'patchwork.views.mail.optout'
-    url = reverse(view)
+
+    def setUp(self):
+        self.url = reverse('patchwork.views.mail.optout')
 
     def testOptOutRequestGET(self):
         response = self.client.get(self.url)
@@ -124,10 +123,9 @@  class OptoutRequestTest(TestCase):
         self.assertEquals(len(mail.outbox), 0)
 
 class OptoutTest(TestCase):
-    view = 'patchwork.views.mail.optout'
-    url = reverse(view)
 
     def setUp(self):
+        self.url = reverse('patchwork.views.mail.optout')
         self.email = u'foo@example.com'
         self.conf = EmailConfirmation(type = 'optout', email = self.email)
         self.conf.save()
@@ -157,10 +155,9 @@  class OptoutPreexistingTest(OptoutTest):
         EmailOptout(email = self.email).save()
 
 class OptinRequestTest(TestCase):
-    view = 'patchwork.views.mail.optin'
-    url = reverse(view)
 
     def setUp(self):
+        self.url = reverse('patchwork.views.mail.optin')
         self.email = u'foo@example.com'
         EmailOptout(email = self.email).save()
 
@@ -232,8 +229,9 @@  class OptinTest(TestCase):
 
 class OptinWithoutOptoutTest(TestCase):
     """Test an opt-in with no existing opt-out"""
-    view = 'patchwork.views.mail.optin'
-    url = reverse(view)
+
+    def setUp(self):
+        self.url = reverse('patchwork.views.mail.optin')
 
     def testOptInWithoutOptout(self):
         email = u'foo@example.com'
@@ -248,16 +246,15 @@  class UserProfileOptoutFormTest(TestCase):
     """Test that the correct optin/optout forms appear on the user profile
        page, for logged-in users"""
 
-    view = 'patchwork.views.user.profile'
-    url = reverse(view)
-    optout_url = reverse('patchwork.views.mail.optout')
-    optin_url = reverse('patchwork.views.mail.optin')
-    form_re_template = ('<form\s+[^>]*action="%(url)s"[^>]*>'
-                        '.*?<input\s+[^>]*value="%(email)s"[^>]*>.*?'
-                        '</form>')
-    secondary_email = 'test2@example.com'
-
     def setUp(self):
+        self.url = reverse('patchwork.views.user.profile')
+        self.optout_url = reverse('patchwork.views.mail.optout')
+        self.optin_url = reverse('patchwork.views.mail.optin')
+        self.form_re_template = ('<form\s+[^>]*action="%(url)s"[^>]*>'
+                                 '.*?<input\s+[^>]*value="%(email)s"[^>]*>.*?'
+                                 '</form>')
+        self.secondary_email = 'test2@example.com'
+
         self.user = create_user()
         self.client.login(username = self.user.username,
                 password = self.user.username)
diff --git a/patchwork/tests/test_notifications.py b/patchwork/tests/test_notifications.py
index c46af61..37adb8d 100644
--- a/patchwork/tests/test_notifications.py
+++ b/patchwork/tests/test_notifications.py
@@ -19,12 +19,10 @@ 
 
 import datetime
 from django.test import TestCase
-from django.core.urlresolvers import reverse
 from django.core import mail
 from django.conf import settings
-from django.db.utils import IntegrityError
 from patchwork.models import Patch, State, PatchChangeNotification, EmailOptout
-from patchwork.tests.utils import defaults, create_maintainer
+from patchwork.tests.utils import defaults
 from patchwork.utils import send_notifications
 
 class PatchNotificationModelTest(TestCase):
diff --git a/patchwork/tests/test_user.py b/patchwork/tests/test_user.py
index 0faa970..d9aef7e 100644
--- a/patchwork/tests/test_user.py
+++ b/patchwork/tests/test_user.py
@@ -17,9 +17,7 @@ 
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-import unittest
 from django.test import TestCase
-from django.test.client import Client
 from django.core import mail
 from django.core.urlresolvers import reverse
 from django.conf import settings
@@ -27,6 +25,7 @@  from django.contrib.auth.models import User
 from patchwork.models import EmailConfirmation, Person, Bundle
 from patchwork.tests.utils import defaults, error_strings
 
+
 def _confirmation_url(conf):
     return reverse('patchwork.views.confirm', kwargs = {'key': conf.key})
 
@@ -121,7 +120,7 @@  class UserPersonConfirmTest(TestCase):
         self.assertEquals(conf.active, False)
 
 class UserLoginRedirectTest(TestCase):
-    
+
     def testUserLoginRedirect(self):
         url = '/user/'
         response = self.client.get(url)
@@ -157,8 +156,12 @@  class UserProfileTest(TestCase):
         self.assertContains(response, bundle.get_absolute_url())
 
 class UserPasswordChangeTest(TestCase):
-    form_url = reverse('django.contrib.auth.views.password_change')
-    done_url = reverse('django.contrib.auth.views.password_change_done')
+    user = None
+
+    def setUp(self):
+        self.form_url = reverse('django.contrib.auth.views.password_change')
+        self.done_url = reverse(
+            'django.contrib.auth.views.password_change_done')
 
     def testPasswordChangeForm(self):
         self.user = TestUser()