launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07417
[Merge] lp:~jcsackett/launchpad/simplify-disclosure-tests into lp:launchpad
j.c.sackett has proposed merging lp:~jcsackett/launchpad/simplify-disclosure-tests into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/simplify-disclosure-tests/+merge/103937
Summary
=======
The great piece of yak shaving you see before you addresses the fact that
TestPillarSharingView, TestPillarSharingDetailsView, and the pillar sharing
views' breadcrumb tests all had nearly identical setups, but were all doing it
separately.
Given anything testing sharing needs to do approximately the same setup
(create a pillar, make some grants) it stands to reason they should have a
unified way to do that, rather than doing it three different ways.
Implementation
==============
A new base class exists for sharing browser based tests. It provides a means
to make policy grantees and artifact grantees, as well as some other setup
common to all the test cases.
Tests
=====
bin/test -vvc -t test_pillar_sharing -t registry.*test_breadcrumbs
QA
==
None, this is just test based yak shaving, b/c duplication offends us all.
LoC
===
This came out at -2 LoC. Which is really sad, given the duplication between
the various testcases. Still, at least the code is DRY.
Lint
====
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/registry/browser/tests/test_breadcrumbs.py
lib/lp/registry/browser/tests/test_pillar_sharing.py
--
https://code.launchpad.net/~jcsackett/launchpad/simplify-disclosure-tests/+merge/103937
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/simplify-disclosure-tests into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_breadcrumbs.py'
--- lib/lp/registry/browser/tests/test_breadcrumbs.py 2012-04-23 21:08:25 +0000
+++ lib/lp/registry/browser/tests/test_breadcrumbs.py 2012-04-27 20:00:36 +0000
@@ -6,50 +6,40 @@
from zope.component import getUtility
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-from lp.registry.model.pillar import PillarPerson
from lp.services.webapp.publisher import canonical_url
from lp.testing import login_person
from lp.testing.breadcrumbs import BaseBreadcrumbTestCase
-
-
-class TestPillarSharingBreadcrumb(BaseBreadcrumbTestCase):
+from lp.registry.browser.tests.test_pillar_sharing import SharingBaseTestCase
+
+
+class TestPillarSharingBreadcrumb(BaseBreadcrumbTestCase, SharingBaseTestCase):
"""Test breadcrumbs for the sharing views."""
+ pillar_type = 'product'
+
def setUp(self):
super(TestPillarSharingBreadcrumb, self).setUp()
- self.pillar = self.factory.makeProduct()
- self.grantee = self.factory.makePerson()
- self.bug = self.factory.makeBug(
- product=self.pillar,
- private=True)
-
- artifact = self.factory.makeAccessArtifact(concrete=self.bug)
- policy = self.factory.makeAccessPolicy(pillar=self.pillar)
- self.factory.makeAccessPolicyArtifact(
- artifact=artifact, policy=policy)
- self.factory.makeAccessArtifactGrant(
- artifact=artifact, grantee=self.grantee, grantor=self.pillar.owner)
- self.pillarperson = PillarPerson(self.pillar, self.grantee)
- login_person(self.pillar.owner)
+ login_person(self.driver)
def test_sharing_breadcrumb(self):
- crumbs = [self.pillar.displayname, 'Sharing']
+ crumbs = [self.pillar.displayname, 'Sharing']
self.assertBreadcrumbTexts(
expected=crumbs,
obj=self.pillar,
view_name="+sharing")
def test_sharing_details_breadcrumbs(self):
+ grantee = self.makeArtifactGrantee()
expected_crumbs = [
self.pillar.displayname,
'Sharing',
- 'Sharing details for %s' % self.grantee.displayname,
+ 'Sharing details for %s' % grantee.displayname,
]
url = 'https://launchpad.dev/%s/+sharing/%s' % (
- self.pillar.name, self.grantee.name)
- crumbs = [c.text for c in self.getBreadcrumbsForUrl(url)]
+ self.pillar.name, grantee.name)
+ crumbs = [c.text for c in self.getBreadcrumbsForUrl(url)]
self.assertEqual(expected_crumbs, crumbs)
- #self.assertBreadcrumbTexts(expected=crumbs, obj=self.pillarperson)
+
class TestDistroseriesBreadcrumb(BaseBreadcrumbTestCase):
"""Test breadcrumbs for an `IDistroseries`."""
=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-04-26 15:52:16 +0000
+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-04-27 20:00:36 +0000
@@ -29,13 +29,13 @@
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
login_person,
+ person_logged_in,
StormStatementRecorder,
TestCaseWithFactory,
)
from lp.testing.layers import DatabaseFunctionalLayer
from lp.testing.matchers import HasQueryCount
from lp.testing.pages import (
- extract_text,
setupBrowserForUser,
)
from lp.testing.views import (
@@ -52,53 +52,92 @@
WRITE_FLAG = {'disclosure.enhanced_sharing.writable': 'true'}
-class PillarSharingDetailsMixin:
- """Test the pillar sharing details view."""
+class SharingBaseTestCase(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def _create_sharing(self, grantee, security=False):
+ pillar_type = None
+
+ def setUp(self):
+ super(SharingBaseTestCase, self).setUp()
+ self.driver = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ if self.pillar_type == 'distribution':
+ self.pillar = self.factory.makeDistribution(
+ owner=self.owner, driver=self.driver)
+ elif self.pillar_type == 'product':
+ self.pillar = self.factory.makeProduct(
+ owner=self.owner, driver=self.driver)
+ self.access_policy = self.factory.makeAccessPolicy(
+ pillar=self.pillar,
+ type=InformationType.PROPRIETARY)
+ self.grantees = []
+
+ def makeGrantee(self, name=None):
+ grantee = self.factory.makePerson(name=name)
+ self.factory.makeAccessPolicyGrant(self.access_policy, grantee)
+ return grantee
+
+ def makeArtifactGrantee(
+ self, grantee=None, with_bug=True,
+ with_branch=False, security=False):
+ if grantee is None:
+ grantee = self.factory.makePerson()
+
+ branch = None
+ bug = None
+ artifacts = []
+
+ if with_branch and self.pillar_type == 'product':
+ branch = self.factory.makeBranch(
+ product=self.pillar,
+ owner=self.pillar.owner,
+ private=True)
+ artifacts.append(
+ self.factory.makeAccessArtifact(concrete=branch))
+
+ if with_bug:
if security:
owner = self.factory.makePerson()
else:
owner = self.pillar.owner
if self.pillar_type == 'product':
- self.bug = self.factory.makeBug(
+ bug = self.factory.makeBug(
product=self.pillar,
owner=owner,
private=True)
- self.branch = self.factory.makeBranch(
- product=self.pillar,
- owner=self.pillar.owner,
- private=True)
elif self.pillar_type == 'distribution':
- self.branch = None
- self.bug = self.factory.makeBug(
+ bug = self.factory.makeBug(
distribution=self.pillar,
owner=owner,
private=True)
- artifact = self.factory.makeAccessArtifact(concrete=self.bug)
- policy = self.factory.makeAccessPolicy(pillar=self.pillar)
+ artifacts.append(
+ self.factory.makeAccessArtifact(concrete=bug))
+
+ for artifact in artifacts:
self.factory.makeAccessPolicyArtifact(
- artifact=artifact, policy=policy)
+ artifact=artifact, policy=self.access_policy)
self.factory.makeAccessArtifactGrant(
- artifact=artifact, grantee=grantee, grantor=self.pillar.owner)
-
- if self.branch:
- artifact = self.factory.makeAccessArtifact(
- concrete=self.branch)
- self.factory.makeAccessPolicyArtifact(
- artifact=artifact, policy=policy)
- self.factory.makeAccessArtifactGrant(
- artifact=artifact, grantee=grantee,
- grantor=self.pillar.owner)
-
- def getPillarPerson(self, person=None, with_sharing=True):
- if person is None:
- person = self.factory.makePerson()
- if with_sharing:
- self._create_sharing(person)
-
+ artifact=artifact,
+ grantee=grantee,
+ grantor=self.pillar.owner)
+ return grantee
+
+ def setupSharing(self, sharees):
+ with person_logged_in(self.owner):
+ # Make grants in ascending order so we can slice off the first
+ # elements in the pillar observer results to check batching.
+ for x in range(10):
+ self.makeArtifactGrantee()
+ grantee = self.makeGrantee('name%s' % x)
+ sharees.append(grantee)
+
+
+class PillarSharingDetailsMixin:
+ """Test the pillar sharing details view."""
+
+ def getPillarPerson(self, person=None, security=False):
+ person = self.makeArtifactGrantee(person, True, True, security)
return PillarPerson(self.pillar, person)
def test_view_filters_security_wisely(self):
@@ -106,13 +145,11 @@
# `launchpad.Driver` -- the permission level for the page -- should be
# able to see.
with FeatureFixture(DETAILS_ENABLED_FLAG):
- pillarperson = self.getPillarPerson(with_sharing=False)
- self._create_sharing(grantee=pillarperson.person, security=True)
+ pillarperson = self.getPillarPerson(security=True)
view = create_initialized_view(pillarperson, '+index')
# The page loads
self.assertEqual(pillarperson.person.displayname, view.page_title)
# The bug, which is not shared with the owner, is not included.
-
self.assertEqual(0, view.shared_bugs_count)
def test_view_traverses_plus_sharingdetails(self):
@@ -135,7 +172,8 @@
with FeatureFixture(DETAILS_ENABLED_FLAG):
# We have to do some fun url hacking to force the traversal a user
# encounters.
- pillarperson = self.getPillarPerson(with_sharing=False)
+ pillarperson = PillarPerson(
+ self.pillar, self.factory.makePerson())
url = 'http://launchpad.dev/%s/+sharing/%s' % (
pillarperson.pillar.name, pillarperson.person.name)
browser = self.getUserBrowser(user=self.owner, url=url)
@@ -162,6 +200,8 @@
with FeatureFixture(DETAILS_ENABLED_FLAG):
pillarperson = self.getPillarPerson()
view = create_initialized_view(pillarperson, '+index')
+ bugtask = list(view.bugs)[0]
+ bug = bugtask.bug
cache = IJSONRequestCache(view.request)
request = get_current_web_service_request()
self.assertEqual({
@@ -171,24 +211,24 @@
self.assertEqual({
'self_link': absoluteURL(pillarperson.pillar, request),
}, cache.objects.get('pillar'))
- bugtask = self.bug.default_bugtask
self.assertEqual({
- 'bug_id': self.bug.id,
- 'bug_summary': self.bug.title,
+ 'bug_id': bug.id,
+ 'bug_summary': bug.title,
'bug_importance': bugtask.importance.title.lower(),
- 'information_type': self.bug.information_type.title,
+ 'information_type': bug.information_type.title,
'web_link': canonical_url(
bugtask, path_only_if_possible=True),
- 'self_link': absoluteURL(self.bug, request),
+ 'self_link': absoluteURL(bug, request),
}, cache.objects.get('bugs')[0])
if self.pillar_type == 'product':
+ branch = list(view.branches)[0]
self.assertEqual({
- 'branch_id': self.branch.id,
- 'branch_name': self.branch.unique_name,
+ 'branch_id': branch.id,
+ 'branch_name': branch.unique_name,
'information_type': InformationType.USERDATA.title,
'web_link': canonical_url(
- self.branch, path_only_if_possible=True),
- 'self_link': absoluteURL(self.branch, request),
+ branch, path_only_if_possible=True),
+ 'self_link': absoluteURL(branch, request),
}, cache.objects.get('branches')[0])
def test_view_write_enabled_without_feature_flag(self):
@@ -209,56 +249,28 @@
class TestProductSharingDetailsView(
- TestCaseWithFactory, PillarSharingDetailsMixin):
+ SharingBaseTestCase, PillarSharingDetailsMixin):
pillar_type = 'product'
def setUp(self):
super(TestProductSharingDetailsView, self).setUp()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeProduct(owner=self.owner)
login_person(self.owner)
class TestDistributionSharingDetailsView(
- TestCaseWithFactory, PillarSharingDetailsMixin):
+ SharingBaseTestCase, PillarSharingDetailsMixin):
pillar_type = 'distribution'
def setUp(self):
super(TestDistributionSharingDetailsView, self).setUp()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeProduct(owner=self.owner)
login_person(self.owner)
class PillarSharingViewTestMixin:
"""Test the PillarSharingView."""
- layer = DatabaseFunctionalLayer
-
- def createSharees(self):
- login_person(self.owner)
- self.access_policy = self.factory.makeAccessPolicy(
- pillar=self.pillar,
- type=InformationType.PROPRIETARY)
- self.grantees = []
-
- def makeGrants(name):
- grantee = self.factory.makePerson(name=name)
- self.grantees.append(grantee)
- # Make access policy grant so that 'All' is returned.
- self.factory.makeAccessPolicyGrant(self.access_policy, grantee)
- # Make access artifact grants so that 'Some' is returned.
- artifact_grant = self.factory.makeAccessArtifactGrant()
- self.factory.makeAccessPolicyArtifact(
- artifact=artifact_grant.abstract_artifact,
- policy=self.access_policy)
- # Make grants for grantees in ascending order so we can slice off the
- # first elements in the pillar observer results to check batching.
- for x in range(10):
- makeGrants('name%s' % x)
-
def test_init_without_feature_flag(self):
# We need a feature flag to enable the view.
self.assertRaises(
@@ -372,28 +384,24 @@
class TestProductSharingView(PillarSharingViewTestMixin,
- TestCaseWithFactory):
+ SharingBaseTestCase):
"""Test the PillarSharingView with products."""
+ pillar_type = 'product'
+
def setUp(self):
super(TestProductSharingView, self).setUp()
- self.driver = self.factory.makePerson()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeProduct(
- owner=self.owner, driver=self.driver)
- self.createSharees()
+ self.setupSharing(self.grantees)
login_person(self.driver)
class TestDistributionSharingView(PillarSharingViewTestMixin,
- TestCaseWithFactory):
+ SharingBaseTestCase):
"""Test the PillarSharingView with distributions."""
+ pillar_type = 'distribution'
+
def setUp(self):
super(TestDistributionSharingView, self).setUp()
- self.driver = self.factory.makePerson()
- self.owner = self.factory.makePerson()
- self.pillar = self.factory.makeDistribution(
- owner=self.owner, driver=self.driver)
- self.createSharees()
+ self.setupSharing(self.grantees)
login_person(self.driver)
Follow ups
