launchpad-reviewers team mailing list archive

[Colin Watson <mp+427763@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:factory-proxy-specification-work-item into launchpad:master.

Commit message:
Return proxied object from makeSpecificationWorkItem

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This required tightening up a few tests.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:factory-proxy-specification-work-item into launchpad:master.

diff --git a/lib/lp/blueprints/browser/tests/test_person_upcomingwork.py b/lib/lp/blueprints/browser/tests/test_person_upcomingwork.py
index c8db194..917012d 100644
--- a/lib/lp/blueprints/browser/tests/test_person_upcomingwork.py
+++ b/lib/lp/blueprints/browser/tests/test_person_upcomingwork.py
@@ -455,22 +455,22 @@ class TestPersonUpcomingWork(BrowserTestCase):
         public_workitem = self.factory.makeSpecificationWorkItem(
             assignee=person, milestone=today_milestone
         )
+        public_spec_name = public_workitem.specification.name
         proprietary_workitem = self.factory.makeSpecificationWorkItem(
             assignee=person,
             milestone=today_milestone,
             specification=proprietary_spec,
         )
+        proprietary_spec_name = removeSecurityProxy(
+            proprietary_workitem
+        ).specification.name
         browser = self.getViewBrowser(person, view_name="+upcomingwork")
-        self.assertIn(public_workitem.specification.name, browser.contents)
-        self.assertNotIn(
-            proprietary_workitem.specification.name, browser.contents
-        )
+        self.assertIn(public_spec_name, browser.contents)
+        self.assertNotIn(proprietary_spec_name, browser.contents)
         browser = self.getViewBrowser(
             person, view_name="+upcomingwork", user=product.owner
         )
-        self.assertIn(
-            proprietary_workitem.specification.name, browser.contents
-        )
+        self.assertIn(proprietary_spec_name, browser.contents)
 
 
 class TestPersonUpcomingWorkView(TestCaseWithFactory):
diff --git a/lib/lp/blueprints/model/tests/test_specification.py b/lib/lp/blueprints/model/tests/test_specification.py
index f383166..79f6b5c 100644
--- a/lib/lp/blueprints/model/tests/test_specification.py
+++ b/lib/lp/blueprints/model/tests/test_specification.py
@@ -8,6 +8,7 @@ from testtools.matchers import Equals, MatchesStructure
 from testtools.testcase import ExpectedException
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
+from zope.security.proxy import isinstance as zope_isinstance
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
@@ -327,7 +328,7 @@ class TestSpecificationWorkItems(TestCaseWithFactory):
     def assertWorkItemsTextContains(self, spec, items):
         expected_lines = []
         for item in items:
-            if isinstance(item, SpecificationWorkItem):
+            if zope_isinstance(item, SpecificationWorkItem):
                 line = ""
                 if item.assignee is not None:
                     line = "[%s] " % item.assignee.name
@@ -675,6 +676,7 @@ class TestSpecificationWorkItems(TestCaseWithFactory):
         work_item = self.factory.makeSpecificationWorkItem()
         spec = work_item.specification
         self.assertEqual(1, len(spec.work_items))
+        login_person(spec.owner)
         spec.updateWorkItems([])
         self.assertEqual(0, len(spec.work_items))
 
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 6d6424c..ac56caf 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2847,7 +2847,7 @@ class LaunchpadObjectFactory(ObjectFactory):
             milestone=milestone,
         )
         work_item.deleted = deleted
-        return work_item
+        return ProxyFactory(work_item)
 
     def makeQuestion(
         self, target=None, title=None, owner=None, description=None, **kwargs