launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/maas-pserv-add-node-pure-papi-tests into lp:maas with lp:~allenap/maas/maas-pserv-add-node as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/maas-pserv-add-node-pure-papi-tests/+merge/92462

Only use the PAPI in maasserver provisioning tests - don't peek into the back of the fakes - so that the tests can be used against a real PAPI.
-- 
https://code.launchpad.net/~allenap/maas/maas-pserv-add-node-pure-papi-tests/+merge/92462
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/maas-pserv-add-node-pure-papi-tests into lp:maas.

=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-02-09 15:32:53 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-02-10 13:00:26 +0000
@@ -18,69 +18,87 @@
 from provisioningserver.testing.fakeapi import FakeSynchronousProvisioningAPI
 
 
-class TestProvisioning(TestCase):
-
-    def patch_in_fake_papi(self):
-        papi_fake = FakeSynchronousProvisioningAPI()
-        patch = MonkeyPatch(
-            "maasserver.provisioning.get_provisioning_api_proxy",
-            lambda: papi_fake)
-        self.useFixture(patch)
-        return papi_fake
-
-    def test_patch_in_fake_papi(self):
-        # patch_in_fake_papi() patches in a fake provisioning API so that we
-        # can observe what the signal handlers are doing.
-        papi = provisioning.get_provisioning_api_proxy()
-        papi_fake = self.patch_in_fake_papi()
-        self.assertIsNot(provisioning.get_provisioning_api_proxy(), papi)
-        self.assertIs(provisioning.get_provisioning_api_proxy(), papi_fake)
-        # The fake has small database, and it's empty to begin with.
-        self.assertEqual({}, papi_fake.distros)
-        self.assertEqual({}, papi_fake.profiles)
-        self.assertEqual({}, papi_fake.nodes)
+class ProvisioningTests:
+    """Tests for the Provisioning API as maasserver sees it."""
+
+    # Must be defined in concrete subclasses.
+    papi = None
 
     def test_provision_post_save_Node_create(self):
         # Creating and saving a node automatically creates a dummy distro and
         # profile too, and associates it with the new node.
-        papi_fake = self.patch_in_fake_papi()
         node_model = Node(system_id="frank")
         provisioning.provision_post_save_Node(
             sender=Node, instance=node_model, created=True)
-        self.assertEqual(["frank"], sorted(papi_fake.nodes))
-        node = papi_fake.nodes["frank"]
+        nodes = self.papi.get_nodes_by_name(["frank"])
+        self.assertEqual(["frank"], sorted(nodes))
+        node = nodes["frank"]
         profile_name = node["profile"]
-        self.assertIn(profile_name, papi_fake.profiles)
-        profile = papi_fake.profiles[profile_name]
+        profiles = self.papi.get_profiles_by_name([profile_name])
+        self.assertEqual([profile_name], sorted(profiles))
+        profile = profiles[profile_name]
         distro_name = profile["distro"]
-        self.assertIn(distro_name, papi_fake.distros)
+        distros = self.papi.get_distros_by_name([distro_name])
+        self.assertEqual([distro_name], sorted(distros))
 
     def test_provision_post_save_Node_update(self):
         # Saving an existing node does not change the profile or distro
         # associated with it.
-        papi_fake = self.patch_in_fake_papi()
         node_model = Node(system_id="frank")
         provisioning.provision_post_save_Node(
             sender=Node, instance=node_model, created=True)
         # Record the current profile name.
-        node = papi_fake.nodes["frank"]
+        node = self.papi.get_nodes_by_name(["frank"])["frank"]
         profile_name1 = node["profile"]
         # Update the model node.
         provisioning.provision_post_save_Node(
             sender=Node, instance=node_model, created=False)
         # The profile name is unchanged.
-        node = papi_fake.nodes["frank"]
+        node = self.papi.get_nodes_by_name(["frank"])["frank"]
         profile_name2 = node["profile"]
         self.assertEqual(profile_name1, profile_name2)
 
     def test_provision_post_delete_Node(self):
-        papi_fake = self.patch_in_fake_papi()
         node_model = Node(system_id="frank")
         provisioning.provision_post_save_Node(
             sender=Node, instance=node_model, created=True)
         provisioning.provision_post_delete_Node(
             sender=Node, instance=node_model)
         # The node is deleted, but the profile and distro remain.
-        self.assertNotEqual({}, papi_fake.distros)
-        self.assertNotEqual({}, papi_fake.profiles)
+        self.assertNotEqual({}, self.papi.get_distros())
+        self.assertNotEqual({}, self.papi.get_profiles())
+        self.assertEqual({}, self.papi.get_nodes_by_name(["frank"]))
+
+
+def patch_in_fake_papi(test):
+    """Patch in a fake Provisioning API for the duration of the test."""
+    papi_fake = FakeSynchronousProvisioningAPI()
+    patch = MonkeyPatch(
+        "maasserver.provisioning.get_provisioning_api_proxy",
+        lambda: papi_fake)
+    test.useFixture(patch)
+    return papi_fake
+
+
+class TestProvisioningFake(TestCase):
+    """Tests for `patch_in_fake_papi`."""
+
+    def test_patch_in_fake_papi(self):
+        # patch_in_fake_papi() patches in a fake provisioning API so that we
+        # can observe what the signal handlers are doing.
+        papi = provisioning.get_provisioning_api_proxy()
+        papi_fake = patch_in_fake_papi(self)
+        self.assertIsNot(provisioning.get_provisioning_api_proxy(), papi)
+        self.assertIs(provisioning.get_provisioning_api_proxy(), papi_fake)
+        # The fake has small database, and it's empty to begin with.
+        self.assertEqual({}, papi_fake.distros)
+        self.assertEqual({}, papi_fake.profiles)
         self.assertEqual({}, papi_fake.nodes)
+
+
+class TestProvisioningWithFake(ProvisioningTests, TestCase):
+    """Tests for the Provisioning API using a fake."""
+
+    def setUp(self):
+        super(TestProvisioningWithFake, self).setUp()
+        self.papi = patch_in_fake_papi(self)