launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/testscenarios-bug-978035 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #978035 in MAAS: "Enlistment tests use mixins to repeat the same tests when different user are logged-in"
  https://bugs.launchpad.net/maas/+bug/978035

For more details, see:
https://code.launchpad.net/~rvb/maas/testscenarios-bug-978035/+merge/106040

This branch refactors test_api.py to use testscenarios (which was recently introduced as a test dependency) instead of a maze of test mixins.

= Pre imp =

This change was discussed with Gavin who is also working on using testscenarios.
-- 
https://code.launchpad.net/~rvb/maas/testscenarios-bug-978035/+merge/106040
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/testscenarios-bug-978035 into lp:maas.

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-05-02 10:01:04 +0000
+++ src/maasserver/tests/test_api.py	2012-05-16 17:56:18 +0000
@@ -12,6 +12,10 @@
 __metaclass__ = type
 __all__ = []
 
+from abc import (
+    ABCMeta,
+    abstractproperty,
+    )
 from base64 import b64encode
 from collections import namedtuple
 from datetime import (
@@ -25,6 +29,7 @@
 import shutil
 
 from django.conf import settings
+from django.contrib.auth.models import AnonymousUser
 from django.db.models.signals import post_save
 from django.http import QueryDict
 from fixtures import Fixture
@@ -146,11 +151,52 @@
             extract_constraints(QueryDict('name=%s' % name)))
 
 
-class EnlistmentAPITest(APIv10TestMixin):
-    # This is a mixin containing enlistement tests.  We will run this for:
-    # an anonymous user, a simple (non-admin) user and an admin user.
-    # XXX: rvb 2012-04-10 bug=978035: It would be better to use
-    # testscenarios for this.
+class MultipleUsersScenarios:
+    """A mixin that uses testscenarios to repeat a testcase as different
+    users.
+
+    The scenarios should inject a `userfactory` variable that will
+    be called to produce the user used in the tests e.g.:
+
+    class ExampleTest(MultipleUsersScenarios, TestCase):
+        scenarios = [
+            ('anon', dict(userfactory=lambda: AnonymousUser())),
+            ('user', dict(userfactory=factory.make_user)),
+            ('admin', dict(userfactory=factory.make_admin)),
+            ]
+
+        def test_something(self):
+            pass
+
+    The test `test_something` with be run 3 times: one with a anonymous user
+    logged in, once with a simple (non-admin) user logged in and once with
+    an admin user logged in.
+    """
+
+    __metaclass__ = ABCMeta
+
+    scenarios = abstractproperty(
+        "The scenarios as defined by testscenarios.")
+
+    def setUp(self):
+        super(MultipleUsersScenarios, self).setUp()
+        user = self.userfactory()
+        if not user.is_anonymous():
+            password = factory.getRandomString()
+            user.set_password(password)
+            user.save()
+            self.logged_in_user = user
+            self.client.login(
+                username=self.logged_in_user.username, password=password)
+
+
+class EnlistmentAPITest(APIv10TestMixin, MultipleUsersScenarios, TestCase):
+    """Enlistement tests."""
+    scenarios = [
+        ('anon', dict(userfactory=lambda: AnonymousUser())),
+        ('user', dict(userfactory=factory.make_user)),
+        ('admin', dict(userfactory=factory.make_admin)),
+        ]
 
     def test_POST_new_creates_node(self):
         # The API allows a Node to be created.
@@ -338,8 +384,14 @@
         self.assertItemsEqual(['architecture'], parsed_result)
 
 
-class NonAdminEnlistmentAPITest(EnlistmentAPITest):
-    # This is a mixin containing enlistement tests for non-admin users.
+class NonAdminEnlistmentAPITest(APIv10TestMixin, MultipleUsersScenarios,
+                                TestCase):
+    # Enlistement tests for non-admin users.
+
+    scenarios = [
+        ('anon', dict(userfactory=lambda: AnonymousUser())),
+        ('user', dict(userfactory=factory.make_user)),
+        ]
 
     def test_POST_non_admin_creates_node_in_declared_state(self):
         # Upon non-admin enlistment, a node goes into the Declared
@@ -362,9 +414,8 @@
             Node.objects.get(system_id=system_id).status)
 
 
-class AnonymousEnlistmentAPITest(NonAdminEnlistmentAPITest, TestCase):
-    # This is an actual test case that uses the NonAdminEnlistmentAPITest
-    # mixin and adds enlistement tests specific to anonymous users.
+class AnonymousEnlistmentAPITest(APIv10TestMixin, TestCase):
+    # Enlistement tests specific to anonymous users.
 
     def test_POST_accept_not_allowed(self):
         # An anonymous user is not allowed to accept an anonymously
@@ -397,10 +448,8 @@
             list(parsed_result))
 
 
-class SimpleUserLoggedInEnlistmentAPITest(NonAdminEnlistmentAPITest,
-                                          LoggedInTestCase):
-    # This is an actual test case that uses the NonAdminEnlistmentAPITest
-    # mixin plus enlistement tests specific to simple (non-admin) users.
+class SimpleUserLoggedInEnlistmentAPITest(APIv10TestMixin, LoggedInTestCase):
+    # Enlistement tests specific to simple (non-admin) users.
 
     def test_POST_accept_not_allowed(self):
         # An non-admin user is not allowed to accept an anonymously
@@ -439,10 +488,8 @@
             list(parsed_result))
 
 
-class AdminLoggedInEnlistmentAPITest(EnlistmentAPITest,
-                                     AdminLoggedInTestCase):
-    # This is an actual test case that uses the EnlistmentAPITest mixin
-    # and adds enlistement tests specific to admin users.
+class AdminLoggedInEnlistmentAPITest(APIv10TestMixin, AdminLoggedInTestCase):
+    # Enlistement tests specific to admin users.
 
     def test_POST_admin_creates_node_in_commissioning_state(self):
         # When an admin user enlists a node, it goes into the