launchpad-reviewers team mailing list archive

[Colin Watson <mp+415115@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:distribution-webservice-tests into launchpad:master.

Commit message:
Stop using launchpadlib in test_distro_webservice

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/415115

This is very slightly faster, and it also helps prepare for adding privacy support to `Distribution` by being more careful about only using attributes of `Distribution` within an interaction.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:distribution-webservice-tests into launchpad:master.

diff --git a/lib/lp/registry/tests/test_distro_webservice.py b/lib/lp/registry/tests/test_distro_webservice.py
index 32c60e0..7216553 100644
--- a/lib/lp/registry/tests/test_distro_webservice.py
+++ b/lib/lp/registry/tests/test_distro_webservice.py
@@ -2,10 +2,9 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from datetime import datetime
+import json
 
-from launchpadlib.errors import Unauthorized
 import pytz
-from zope.security.management import endInteraction
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
@@ -20,11 +19,12 @@ from lp.code.model.seriessourcepackagebranch import (
 from lp.registry.interfaces.person import TeamMembershipPolicy
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.testing import (
+    anonymous_logged_in,
     api_url,
-    launchpadlib_for,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import webservice_for_person
 
 
 class TestDistribution(TestCaseWithFactory):
@@ -34,11 +34,11 @@ class TestDistribution(TestCaseWithFactory):
 
     def test_write_without_permission_gives_Unauthorized(self):
         distro = self.factory.makeDistribution()
-        endInteraction()
-        lp = launchpadlib_for("anonymous-access")
-        lp_distro = lp.load(api_url(distro))
-        lp_distro.active = False
-        self.assertRaises(Unauthorized, lp_distro.lp_save)
+        distro_url = api_url(distro)
+        ws = webservice_for_person(None, default_api_version="devel")
+        response = ws.patch(
+            distro_url, "application/json", json.dumps({"active": False}))
+        self.assertEqual(401, response.status)
 
 
 class TestGetBranchTips(TestCaseWithFactory):
@@ -49,6 +49,8 @@ class TestGetBranchTips(TestCaseWithFactory):
     def setUp(self):
         super().setUp()
         self.distro = self.factory.makeDistribution()
+        self.distro_name = self.distro.name
+        self.distro_url = api_url(self.distro)
         series_1 = self.series_1 = self.factory.makeDistroSeries(self.distro)
         series_2 = self.series_2 = self.factory.makeDistroSeries(self.distro)
         source_package = self.factory.makeSourcePackage(distroseries=series_1)
@@ -68,15 +70,15 @@ class TestGetBranchTips(TestCaseWithFactory):
         self.branch_name = branch.unique_name
         self.unofficial_branch_name = unofficial_branch.unique_name
         self.branch_last_scanned_id = branch.last_scanned_id
-        endInteraction()
-        self.lp = launchpadlib_for("anonymous-access")
-        self.lp_distro = self.lp.distributions[self.distro.name]
+        self.ws = webservice_for_person(None, default_api_version="devel")
 
     def test_structure(self):
         """The structure of the results is what we expect."""
         # The results should be structured as a list of
         # (location, tip revision ID, [official series, official series, ...])
-        item = self.lp_distro.getBranchTips()[0]
+        response = self.ws.named_get(self.distro_url, "getBranchTips")
+        self.assertEqual(200, response.status)
+        item = response.jsonBody()[0]
         self.assertEqual(item[0], self.branch_name)
         self.assertTrue(item[1], self.branch_last_scanned_id)
         self.assertEqual(
@@ -87,34 +89,49 @@ class TestGetBranchTips(TestCaseWithFactory):
         """Calling getBranchTips directly matches calling it via the API."""
         # The web service transmutes tuples into lists, so we have to do the
         # same to the results of directly calling getBranchTips.
-        listified = [list(x) for x in self.distro.getBranchTips()]
-        self.assertEqual(listified, self.lp_distro.getBranchTips())
+        with anonymous_logged_in():
+            listified = [list(x) for x in self.distro.getBranchTips()]
+        response = self.ws.named_get(self.distro_url, "getBranchTips")
+        self.assertEqual(200, response.status)
+        self.assertEqual(listified, response.jsonBody())
 
     def test_revisions(self):
         """If a branch has revisions then the most recent one is returned."""
-        revision = self.lp_distro.getBranchTips()[0][1]
+        response = self.ws.named_get(self.distro_url, "getBranchTips")
+        self.assertEqual(200, response.status)
+        revision = response.jsonBody()[0][1]
         self.assertNotEqual(None, revision)
 
     def test_since(self):
         """If "since" is given, return branches with new tips since then."""
         # There is at least one branch with a tip since the year 2000.
-        self.assertNotEqual(0, len(self.lp_distro.getBranchTips(
-            since=datetime(2000, 1, 1))))
+        response = self.ws.named_get(
+            self.distro_url, "getBranchTips",
+            since=datetime(2000, 1, 1).isoformat())
+        self.assertEqual(200, response.status)
+        self.assertNotEqual(0, len(response.jsonBody()))
         # There are no branches with a tip since the year 3000.
-        self.assertEqual(0, len(self.lp_distro.getBranchTips(
-            since=datetime(3000, 1, 1))))
+        response = self.ws.named_get(
+            self.distro_url, "getBranchTips",
+            since=datetime(3000, 1, 1).isoformat())
+        self.assertEqual(200, response.status)
+        self.assertEqual(0, len(response.jsonBody()))
 
     def test_series(self):
         """The official series are included in the data."""
         actual_series_names = sorted([self.series_1.name, self.series_2.name])
-        returned_series_names = sorted(self.lp_distro.getBranchTips()[0][-1])
+        response = self.ws.named_get(self.distro_url, "getBranchTips")
+        self.assertEqual(200, response.status)
+        returned_series_names = sorted(response.jsonBody()[0][-1])
         self.assertEqual(actual_series_names, returned_series_names)
 
     def test_unofficial_branch(self):
         """Not all branches are official."""
         # If a branch isn't official, the last skanned ID will be None and the
         # official distro series list will be empty.
-        tips = self.lp_distro.getBranchTips()[1]
+        response = self.ws.named_get(self.distro_url, "getBranchTips")
+        self.assertEqual(200, response.status)
+        tips = response.jsonBody()[1]
         self.assertEqual(tips[0], self.unofficial_branch_name)
         self.assertEqual(tips[1], None)
         self.assertEqual(tips[2], [])