launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/testfix-spec-security into lp:launchpad.

Commit message:
Fix various attribute access failures in tests.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/testfix-spec-security/+merge/223707

Fix various attribute access failures in tests.
-- 
https://code.launchpad.net/~cjwatson/launchpad/testfix-spec-security/+merge/223707
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/testfix-spec-security into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/tests/test_person_upcomingwork.py'
--- lib/lp/blueprints/browser/tests/test_person_upcomingwork.py	2014-06-17 06:13:08 +0000
+++ lib/lp/blueprints/browser/tests/test_person_upcomingwork.py	2014-06-19 10:17:00 +0000
@@ -17,10 +17,7 @@
     getWorkItemsDueBefore,
     WorkItemContainer,
     )
-from lp.blueprints.enums import (
-    SpecificationPriority,
-    SpecificationWorkItemStatus,
-    )
+from lp.blueprints.enums import SpecificationWorkItemStatus
 from lp.testing import (
     anonymous_logged_in,
     BrowserTestCase,
@@ -394,8 +391,9 @@
         person = self.factory.makePerson()
         proprietary_spec = self.factory.makeSpecification(
             information_type=InformationType.PROPRIETARY)
+        product = removeSecurityProxy(proprietary_spec).product
         today_milestone = self.factory.makeMilestone(
-            dateexpected=self.today, product=proprietary_spec.product)
+            dateexpected=self.today, product=product)
         public_workitem = self.factory.makeSpecificationWorkItem(
             assignee=person, milestone=today_milestone)
         proprietary_workitem = self.factory.makeSpecificationWorkItem(
@@ -407,8 +405,7 @@
         self.assertNotIn(proprietary_workitem.specification.name,
                          browser.contents)
         browser = self.getViewBrowser(
-            person, view_name='+upcomingwork',
-            user=proprietary_workitem.specification.product.owner)
+            person, view_name='+upcomingwork', user=product.owner)
         self.assertIn(proprietary_workitem.specification.name,
                       browser.contents)
 

=== modified file 'lib/lp/blueprints/browser/tests/test_specificationdependency.py'
--- lib/lp/blueprints/browser/tests/test_specificationdependency.py	2013-01-25 03:30:08 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationdependency.py	2014-06-19 10:17:00 +0000
@@ -92,7 +92,8 @@
         proprietary_blocked = self.factory.makeBlueprint(
             product=product, information_type=InformationType.PROPRIETARY)
         public_blocked = self.factory.makeBlueprint(product=product)
-        proprietary_blocked.createDependency(root)
+        with person_logged_in(product.owner):
+            proprietary_blocked.createDependency(root)
         public_blocked.createDependency(root)
 
         # Anonymous can see only the public

=== modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py'
--- lib/lp/blueprints/browser/tests/test_specificationtarget.py	2014-02-19 04:01:46 +0000
+++ lib/lp/blueprints/browser/tests/test_specificationtarget.py	2014-06-19 10:17:00 +0000
@@ -324,15 +324,16 @@
         # Other users see the page, but not the private specs.
         proprietary = self.factory.makeSpecification(
             information_type=InformationType.PROPRIETARY)
-        product = proprietary.product
+        product = removeSecurityProxy(proprietary).product
         public = self.factory.makeSpecification(product=product)
         with person_logged_in(product.owner):
             product.blueprints_usage = ServiceUsage.LAUNCHPAD
             browser = self.getViewBrowser(product, '+specs')
         self.assertIn(public.name, browser.contents)
-        self.assertNotIn(proprietary.name, browser.contents)
+        self.assertNotIn(
+            removeSecurityProxy(proprietary).name, browser.contents)
         with person_logged_in(None):
             browser = self.getViewBrowser(product, '+specs',
                                           user=product.owner)
         self.assertIn(public.name, browser.contents)
-        self.assertIn(proprietary.name, browser.contents)
+        self.assertIn(removeSecurityProxy(proprietary).name, browser.contents)

=== modified file 'lib/lp/blueprints/browser/tests/test_sprint.py'
--- lib/lp/blueprints/browser/tests/test_sprint.py	2013-01-31 05:58:03 +0000
+++ lib/lp/blueprints/browser/tests/test_sprint.py	2014-06-19 10:17:00 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 from testtools.matchers import Equals
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
 from lp.testing import BrowserTestCase
@@ -46,7 +47,8 @@
         for count in range(10):
             blueprint = self.factory.makeSpecification(
                 information_type=InformationType.PROPRIETARY)
-            link = blueprint.linkSprint(sprint, blueprint.owner)
+            owner = removeSecurityProxy(blueprint).owner
+            link = removeSecurityProxy(blueprint).linkSprint(sprint, owner)
             link.acceptBy(sprint.owner)
         with QueryCollector() as recorder:
             self.getViewBrowser(sprint)

=== modified file 'lib/lp/blueprints/model/tests/test_specification.py'
--- lib/lp/blueprints/model/tests/test_specification.py	2013-12-13 12:51:41 +0000
+++ lib/lp/blueprints/model/tests/test_specification.py	2014-06-19 10:17:00 +0000
@@ -140,7 +140,8 @@
         proprietary_blocked = self.factory.makeBlueprint(
             product=product, information_type=InformationType.PROPRIETARY)
         public_blocked = self.factory.makeBlueprint(product=product)
-        proprietary_blocked.createDependency(root)
+        with person_logged_in(owner):
+            proprietary_blocked.createDependency(root)
         public_blocked.createDependency(root)
         # Anonymous (no user) requests only get public blocked specs.
         self.assertEqual(

=== modified file 'lib/lp/blueprints/model/tests/test_sprint.py'
--- lib/lp/blueprints/model/tests/test_sprint.py	2014-06-17 06:13:08 +0000
+++ lib/lp/blueprints/model/tests/test_sprint.py	2014-06-19 10:17:00 +0000
@@ -47,9 +47,10 @@
         blueprint = self.factory.makeSpecification(
             title=title, status=status, name=name,
             information_type=information_type)
+        owner = removeSecurityProxy(blueprint).owner
         if priority is not None:
             removeSecurityProxy(blueprint).priority = priority
-        link = blueprint.linkSprint(sprint, blueprint.owner)
+        link = removeSecurityProxy(blueprint).linkSprint(sprint, owner)
         naked_link = removeSecurityProxy(link)
         if declined:
             link.declineBy(sprint.owner)
@@ -193,29 +194,29 @@
         # Proprietary blueprints are not listed for random users
         blueprint1 = self.makeSpec(
             information_type=InformationType.PROPRIETARY)
-        self.assertEqual([], list_result(blueprint1.sprints[0]))
+        sprint = removeSecurityProxy(blueprint1).sprints[0]
+        self.assertEqual([], list_result(sprint))
 
     def test_proprietary_listed_for_artifact_grant(self):
         # Proprietary blueprints are listed for users with an artifact grant.
         blueprint1 = self.makeSpec(
             information_type=InformationType.PROPRIETARY)
+        sprint = removeSecurityProxy(blueprint1).sprints[0]
         grant = self.factory.makeAccessArtifactGrant(
             concrete_artifact=blueprint1)
-        self.assertEqual(
-            [blueprint1],
-            list_result(blueprint1.sprints[0], user=grant.grantee))
+        self.assertEqual([blueprint1], list_result(sprint, user=grant.grantee))
 
     def test_proprietary_listed_for_policy_grant(self):
         # Proprietary blueprints are listed for users with a policy grant.
         blueprint1 = self.makeSpec(
             information_type=InformationType.PROPRIETARY)
+        sprint = removeSecurityProxy(blueprint1).sprints[0]
         policy_source = getUtility(IAccessPolicySource)
         (policy,) = policy_source.find(
-            [(blueprint1.product, InformationType.PROPRIETARY)])
+            [(removeSecurityProxy(blueprint1).product,
+              InformationType.PROPRIETARY)])
         grant = self.factory.makeAccessPolicyGrant(policy)
-        self.assertEqual(
-            [blueprint1],
-            list_result(blueprint1.sprints[0], user=grant.grantee))
+        self.assertEqual([blueprint1], list_result(sprint, user=grant.grantee))
 
 
 class TestSprintAttendancesSort(TestCaseWithFactory):

=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py	2014-06-19 03:31:36 +0000
+++ lib/lp/blueprints/tests/test_specification.py	2014-06-19 10:17:00 +0000
@@ -522,13 +522,15 @@
             spec.drafter = drafter
             address = owner.preferredemail.email
             drafter_address = drafter.preferredemail.email
-        self.assertContentEqual(
-            [address, drafter_address], spec.notificationRecipientAddresses())
+            self.assertContentEqual(
+                [address, drafter_address],
+                spec.notificationRecipientAddresses())
         # Remove the drafters access to the spec.
         artifact = self.factory.makeAccessArtifact(concrete=spec)
         getUtility(IAccessArtifactGrantSource).revokeByArtifact(
             [artifact], [drafter])
-        self.assertEqual([address], spec.notificationRecipientAddresses())
+        with person_logged_in(owner):
+            self.assertEqual([address], spec.notificationRecipientAddresses())
 
 
 class TestSpecificationSet(TestCaseWithFactory):
@@ -761,7 +763,7 @@
         # Proprietary blueprints are listed for users with a policy grant.
         spec = self.makeSpec(information_type=InformationType.PROPRIETARY)
         (policy,) = getUtility(IAccessPolicySource).find(
-            [(spec.product, InformationType.PROPRIETARY)])
+            [(removeSecurityProxy(spec).product, InformationType.PROPRIETARY)])
         grant = self.factory.makeAccessPolicyGrant(policy)
         self._assertInSpecifications(spec, grant)
 

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2014-06-09 22:32:44 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2014-06-19 10:17:00 +0000
@@ -674,7 +674,7 @@
         bug = self.factory.makeBug()
         spec = self.factory.makeSpecification(
             information_type=InformationType.PROPRIETARY)
-        with person_logged_in(spec.product.owner):
+        with person_logged_in(removeSecurityProxy(spec).product.owner):
             spec.linkBug(bug)
         return spec, bug
 
@@ -690,7 +690,8 @@
     def test_bug_specifications_for_authorised_user(self):
         spec, bug = self._createBugAndSpecification()
         self.assertContentEqual(
-            [spec], bug.getSpecifications(spec.product.owner))
+            [spec],
+            bug.getSpecifications(removeSecurityProxy(spec).product.owner))
 
 
 class TestBugTaskDelta(TestCaseWithFactory):

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2014-05-20 18:00:32 +0000
+++ lib/lp/code/model/tests/test_branch.py	2014-06-19 10:17:00 +0000
@@ -1266,7 +1266,7 @@
     def test_specBranchLinkDisablesDeletion(self):
         """A branch linked to a spec cannot be deleted."""
         spec = getUtility(ISpecificationSet).new(
-            name='some-spec', title='Some spec', product=self.product,
+            name='some-spec', title='Some spec', target=self.product,
             owner=self.user, summary='', specurl=None,
             definition_status=NewSpecificationDefinitionStatus.NEW)
         spec.linkBranch(self.branch, self.user)

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2014-06-19 06:38:53 +0000
+++ lib/lp/registry/tests/test_product.py	2014-06-19 10:17:00 +0000
@@ -2131,14 +2131,13 @@
         # Proprietary blueprints are listed for users with a policy grant.
         blueprint1 = self.makeSpec(
             information_type=InformationType.PROPRIETARY)
+        product = removeSecurityProxy(blueprint1).product
         policy_source = getUtility(IAccessPolicySource)
         (policy,) = policy_source.find(
-            [(blueprint1.product, InformationType.PROPRIETARY)])
+            [(product, InformationType.PROPRIETARY)])
         grant = self.factory.makeAccessPolicyGrant(policy)
         self.assertEqual(
-            [blueprint1],
-            list_result(removeSecurityProxy(blueprint1).product,
-                        user=grant.grantee))
+            [blueprint1], list_result(product, user=grant.grantee))
 
 
 class TestWebService(WebServiceTestCase):