launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01564
[Merge] lp:~deryck/launchpad/better-testing-for-status-changes into lp:launchpad/devel
Deryck Hodge has proposed merging lp:~deryck/launchpad/better-testing-for-status-changes into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#126516 would like limited access control for status field changes
https://bugs.launchpad.net/bugs/126516
This branch is the first toward a fix for Bug #126516, or more specifically for a fix to limit transitioning away from Fix Released status. Before this branch the tests for bugtask status changes was a mix of a unit test and doctest, both living under lp.bugs.tests.
The branch refactors that mess, opting to do a complete unit test of BugTask.transitionToStatus and BugTask.canTransitionToStatus. The doctest is then made to be documentation and moved to lp.bugs.doc. Some example code is left in the doctest, though refactored to use factory generated data.
To test, run:
./bin/test -cvvt bugtask-status-changes
./bin/test -cvvt test_bugtask_status
Thanks for the review!
--
https://code.launchpad.net/~deryck/launchpad/better-testing-for-status-changes/+merge/38552
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/better-testing-for-status-changes into lp:launchpad/devel.
=== renamed file 'lib/lp/bugs/tests/test_bugtask_status.txt' => 'lib/lp/bugs/doc/bugtask-status-changes.txt'
--- lib/lp/bugs/tests/test_bugtask_status.txt 2010-10-03 15:30:06 +0000
+++ lib/lp/bugs/doc/bugtask-status-changes.txt 2010-10-15 15:31:11 +0000
@@ -1,137 +1,57 @@
-= Setting Bug Statuses =
-
- >>> from canonical.launchpad.interfaces import (
- ... CreateBugParams, IPersonSet, IProductSet)
- >>> from canonical.launchpad.interfaces import BugTaskStatus
-
- >>> firefox = getUtility(IProductSet).getByName('firefox')
- >>> nopriv_user = getUtility(IPersonSet).getByName('no-priv')
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> firefox.setBugSupervisor(nopriv_user, nopriv_user)
- >>> login('no-priv@xxxxxxxxxxxxx')
- >>> bug_params = CreateBugParams(
- ... nopriv_user, 'Test bug', 'Something')
- >>> firefox_bug = firefox.createBug(bug_params)
-
-Bug Supervisors can transition bugs to the Won't Fix, Expired and
-Triaged statuses.
-
- >>> [firefox_bugtask] = firefox_bug.bugtasks
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.WONTFIX, nopriv_user)
- >>> print firefox_bugtask.status.title
- Won't Fix
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.EXPIRED, nopriv_user)
- >>> print firefox_bugtask.status.title
- Expired
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.TRIAGED, nopriv_user)
- >>> print firefox_bugtask.status.title
- Triaged
-
-The product registrant can transition to the Won't Fix, Expired and
-Triaged statuses too.
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.CONFIRMED, nopriv_user)
- >>> print firefox_bugtask.status.title
- Confirmed
-
- >>> firefox.owner.inTeam(firefox.bug_supervisor)
- False
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.WONTFIX, firefox.owner)
- >>> print firefox_bugtask.status.title
- Won't Fix
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.EXPIRED, firefox.owner)
- >>> print firefox_bugtask.status.title
- Expired
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.TRIAGED, firefox.owner)
- >>> print firefox_bugtask.status.title
- Triaged
-
-Users who are not bug supervisors or product registrants cannot
-transition the status to Won't Fix or Triaged. The option is not
-exposed in the UI, but we also enforce this rule at the database level
-to ensure that all call sites adhere to this.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> firefox.setBugSupervisor(None, None)
-
- >>> login('no-priv@xxxxxxxxxxxxx')
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.WONTFIX, nopriv_user)
- Traceback (most recent call last):
- ...
- UserCannotEditBugTaskStatus: Only Bug Supervisors may change status to Won't Fix.
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.EXPIRED, nopriv_user)
- Traceback (most recent call last):
- ...
- UserCannotEditBugTaskStatus: Only Bug Supervisors may change status to Expired.
-
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.TRIAGED, nopriv_user)
- Traceback (most recent call last):
- ...
- UserCannotEditBugTaskStatus: Only Bug Supervisors may change status to Triaged.
-
-Users who are not bug supervisors or product registrants cannot
-transition the status from Won't Fix to anything else.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> foo_bar = getUtility(ILaunchBag).user
- >>> firefox.setBugSupervisor(foo_bar, foo_bar)
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.WONTFIX, foo_bar)
-
- >>> login('no-priv@xxxxxxxxxxxxx')
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.NEW, nopriv_user)
- Traceback (most recent call last):
- ...
- UserCannotEditBugTaskStatus: Only Bug Supervisors may change status to New.
+Changing Bug Task Status
+========================
+
+Restrictions
+------------
+
+There are a few simple rules around who can change the status of a
+bug task. There are three statuses that can only be set by either
+the project registrant or the bug supervisor:
+
+ * Won't Fix
+ * Expired
+ * Triaged
+
+ >>> owner = factory.makePerson()
+ >>> product = factory.makeProduct(owner=owner)
+ >>> bugtask = factory.makeBugTask(target=product)
+ >>> user = factory.makePerson()
+
+ >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
+ >>> login_person(owner)
+ >>> bugtask.transitionToStatus(BugTaskStatus.WONTFIX, owner)
+ >>> print bugtask.status.title
+ Won't Fix
+
+Regular users of Launchpad cannot transition a bug task to any of
+these statuses.
+
+An additional restraint is added to Won't Fix. Only the product
+registrant or bug supervisor can change from this status to any
+other status.
+
+ >>> login_person(user)
+ >>> bugtask.transitionToStatus(BugTaskStatus.CONFIRMED, user)
+ Traceback (most recent call last):
+ ...
+ UserCannotEditBugTaskStatus...
+
+This is fully tested in
+lp.bugs.tests.test_bugtask_status.TestBugTaskStatusSetting.
+
+Testing for Permission
+----------------------
The method IBugTask.canTransitionToStatus comes in handy here. It
tells us if a transition to a status is permitted. It is *not* a
-dry-run of transitionToStatus, but is good enough and fast enough to
-be used by UI code, e.g. to display only those statuses to which a
-user can transition a particular bugtask.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.TRIAGED, foo_bar)
-
- >>> login('no-priv@xxxxxxxxxxxxx')
-
- >>> firefox_bugtask.canTransitionToStatus(
- ... BugTaskStatus.WONTFIX, nopriv_user)
- False
-
- >>> firefox_bugtask.canTransitionToStatus(
- ... BugTaskStatus.TRIAGED, nopriv_user)
- False
-
- >>> firefox_bugtask.canTransitionToStatus(
- ... BugTaskStatus.INCOMPLETE, nopriv_user)
+dry-run of IBugTask.transitionToStatus, but is good enough and fast
+enough to be used by UI code, e.g. to display only those statuses to
+which a user can transition a particular bugtask.
+
+ >>> bugtask.canTransitionToStatus(BugTaskStatus.TRIAGED, owner)
True
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> firefox_bugtask.transitionToStatus(
- ... BugTaskStatus.WONTFIX, getUtility(ILaunchBag).user)
-
- >>> login('no-priv@xxxxxxxxxxxxx')
-
- >>> firefox_bugtask.canTransitionToStatus(
- ... BugTaskStatus.NEW, nopriv_user)
+ >>> bugtask.canTransitionToStatus(BugTaskStatus.TRIAGED, user)
False
+
+This method is fully tested in
+lp.bugs.tests.test_bugtask_status.TestCanTransitionToStatus.
=== modified file 'lib/lp/bugs/tests/test_bugtask_status.py'
--- lib/lp/bugs/tests/test_bugtask_status.py 2010-10-04 19:50:45 +0000
+++ lib/lp/bugs/tests/test_bugtask_status.py 2010-10-15 15:31:11 +0000
@@ -1,22 +1,261 @@
# Copyright 2009 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Test for choosing the request and publication."""
+"""Tests for bug task status transitions."""
__metaclass__ = type
-from canonical.launchpad.testing.systemdocs import (
- LayeredDocFileSuite,
- setUp,
- tearDown,
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.bugs.interfaces.bugtask import UserCannotEditBugTaskStatus
+from lp.bugs.model.bugtask import BugTaskStatus
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
)
-from canonical.testing.layers import LaunchpadFunctionalLayer
-
-
-def test_suite():
- suite = LayeredDocFileSuite(
- 'test_bugtask_status.txt',
- layer=LaunchpadFunctionalLayer, setUp=setUp, tearDown=tearDown,
- )
- return suite
-
+
+
+class TestBugTaskStatusSetting(TestCaseWithFactory):
+ """Tests to ensure restricted status changes are enforced."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ # We can work with a project only here, since both project
+ # and distribution use the same methods on IBugTask.
+ super(TestBugTaskStatusSetting, self).setUp()
+ self.owner = self.factory.makePerson()
+ self.team_member = self.factory.makePerson()
+ self.supervisor = self.factory.makeTeam(owner=self.owner)
+ self.product = self.factory.makeProduct(owner=self.owner)
+ self.task = self.factory.makeBugTask(target=self.product)
+ self.bug = self.task.bug
+ with person_logged_in(self.owner):
+ self.supervisor.addMember(self.team_member, self.owner)
+ self.product.setBugSupervisor(self.supervisor, self.owner)
+
+ def test_person_cannot_set_bug_supervisor_statuses(self):
+ # A regular user should not be able to set statuses in
+ # BUG_SUPERVISOR_BUGTASK_STATUSES.
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ self.assertRaises(
+ UserCannotEditBugTaskStatus, self.task.transitionToStatus,
+ BugTaskStatus.WONTFIX, person)
+ self.assertRaises(
+ UserCannotEditBugTaskStatus, self.task.transitionToStatus,
+ BugTaskStatus.EXPIRED, person)
+ self.assertRaises(
+ UserCannotEditBugTaskStatus, self.task.transitionToStatus,
+ BugTaskStatus.TRIAGED, person)
+
+ def test_owner_can_set_bug_supervisor_statuses(self):
+ # Project registrant should be able to set statuses in
+ # BUG_SUPERVISOR_BUGTASK_STATUSES.
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ with person_logged_in(self.owner):
+ self.task.transitionToStatus(BugTaskStatus.WONTFIX, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ self.task.transitionToStatus(BugTaskStatus.EXPIRED, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.EXPIRED)
+ self.task.transitionToStatus(BugTaskStatus.TRIAGED, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.TRIAGED)
+
+ def test_supervisor_can_set_bug_supervisor_statuses(self):
+ # Bug supervisor should be able to set statuses in
+ # BUG_SUPERVISOR_BUGTASK_STATUSES.
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ with person_logged_in(self.team_member):
+ self.task.transitionToStatus(
+ BugTaskStatus.WONTFIX, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ self.task.transitionToStatus(
+ BugTaskStatus.EXPIRED, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.EXPIRED)
+ self.task.transitionToStatus(
+ BugTaskStatus.TRIAGED, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.TRIAGED)
+
+ def test_unset_wont_fix_status(self):
+ # A regular user should not be able to transition a bug away
+ # from Won't Fix. Bug supervisor and owner should be able
+ # to transition away from Won't Fix.
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ with person_logged_in(self.team_member):
+ self.task.transitionToStatus(
+ BugTaskStatus.WONTFIX, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ self.task.transitionToStatus(
+ BugTaskStatus.NEW, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ with person_logged_in(self.owner):
+ self.task.transitionToStatus(
+ BugTaskStatus.WONTFIX, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ self.task.transitionToStatus(
+ BugTaskStatus.NEW, self.team_member)
+ self.assertEqual(self.task.status, BugTaskStatus.NEW)
+ # Transition back to Won't Fix for final assert below.
+ self.task.transitionToStatus(
+ BugTaskStatus.WONTFIX, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ self.assertRaises(
+ UserCannotEditBugTaskStatus, self.task.transitionToStatus,
+ BugTaskStatus.CONFIRMED, person)
+
+
+class TestCanTransitionToStatus(TestCaseWithFactory):
+ """Tests for BugTask.canTransitionToStatus."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ # We can work with a project only here, since both project
+ # and distribution use the same methods on IBugTask.
+ super(TestCanTransitionToStatus, self).setUp()
+ self.user = self.factory.makePerson()
+ self.owner = self.factory.makePerson()
+ self.team_member = self.factory.makePerson()
+ self.supervisor = self.factory.makeTeam(owner=self.owner)
+ self.product = self.factory.makeProduct(owner=self.owner)
+ self.task = self.factory.makeBugTask(target=self.product)
+ self.bug = self.task.bug
+ with person_logged_in(self.owner):
+ self.supervisor.addMember(self.team_member, self.owner)
+ self.product.setBugSupervisor(self.supervisor, self.owner)
+
+ def test_user_cannot_transition_bug_supervisor_statuses(self):
+ # A regular user is not allowed to transition to
+ # BUG_SUPERVISOR_BUGTASK_STATUSES.
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.WONTFIX, self.user),
+ False)
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.EXPIRED, self.user),
+ False)
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.TRIAGED, self.user),
+ False)
+
+ def test_user_can_transition_to_any_other_status(self):
+ # A regular user should be able to transition to any status
+ # other than those in BUG_SUPERVISOR_BUGTASK_STATUSES.
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.NEW, self.user),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INCOMPLETE, self.user), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.OPINION, self.user),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.INVALID, self.user),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.CONFIRMED, self.user), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INPROGRESS, self.user), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXCOMMITTED, self.user), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXRELEASED, self.user), True)
+
+ def test_bug_supervisor_can_transition_to_any_status(self):
+ # A bug supervisor can transition to any status.
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.WONTFIX, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.EXPIRED, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.TRIAGED, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.NEW, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INCOMPLETE, self.supervisor), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.OPINION, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INVALID, self.supervisor),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.CONFIRMED, self.supervisor), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INPROGRESS, self.supervisor), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXCOMMITTED, self.supervisor), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXRELEASED, self.supervisor), True)
+
+ def test_owner_can_transition_to_any_status(self):
+ # An owner can transition to any status.
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.WONTFIX, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.EXPIRED, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.TRIAGED, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.NEW, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INCOMPLETE, self.owner), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.OPINION, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INVALID, self.owner),
+ True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.CONFIRMED, self.owner), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.INPROGRESS, self.owner), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXCOMMITTED, self.owner), True)
+ self.assertEqual(
+ self.task.canTransitionToStatus(
+ BugTaskStatus.FIXRELEASED, self.owner), True)
+
+ def test_user_cannot_transition_from_wont_fix(self):
+ # A regular user cannot transition away from Won't Fix.
+ with person_logged_in(self.owner):
+ self.task.transitionToStatus(BugTaskStatus.WONTFIX, self.owner)
+ self.assertEqual(self.task.status, BugTaskStatus.WONTFIX)
+ self.assertEqual(
+ self.task.canTransitionToStatus(BugTaskStatus.NEW, self.user),
+ False)
Follow ups
