Merge ~pappacena/launchpad:vcs-scenarios-for-bmp into launchpad:master

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 04850f499a3fda3adeacdd816d91baab3d0c965e
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:vcs-scenarios-for-bmp
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:bugfix-private-mp-webhook-trigger
Diff against target: 582 lines (+124/-137)
2 files modified
lib/lp/code/model/tests/test_branchmergeproposal.py (+111/-133)
lib/lp/code/tests/helpers.py (+13/-4)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+387878@code.launchpad.net

Commit message

Refactoring BMP test cases to extend scenarios on TestBranchMergeProposalNominateReviewer

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index 7e380a3..0d2457d 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -57,6 +57,7 @@ from lp.code.event.branchmergeproposal import (
57 BranchMergeProposalNeedsReviewEvent,57 BranchMergeProposalNeedsReviewEvent,
58 ReviewerNominatedEvent,58 ReviewerNominatedEvent,
59 )59 )
60from lp.code.interfaces.branch import IBranch
60from lp.code.interfaces.branchmergeproposal import (61from lp.code.interfaces.branchmergeproposal import (
61 BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,62 BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
62 BRANCH_MERGE_PROPOSAL_OBSOLETE_STATES as OBSOLETE_STATES,63 BRANCH_MERGE_PROPOSAL_OBSOLETE_STATES as OBSOLETE_STATES,
@@ -110,6 +111,49 @@ from lp.testing.layers import (
110 )111 )
111112
112113
114class WithVCSScenarios(WithScenarios):
115
116 scenarios = [
117 ("bzr", {"git": False}),
118 ("git", {"git": True}),
119 ]
120
121 def makeBranch(self, same_target_as=None, product=None, stacked_on=None,
122 information_type=None, owner=None):
123 # Create the product pillar and its access policy if information
124 # type is "PROPRIETARY".
125 if product is None and same_target_as is None:
126 product = self.factory.makeProduct()
127 if information_type == InformationType.PROPRIETARY:
128 self.factory.makeAccessPolicy(product)
129 elif product is None:
130 same_target_as = removeSecurityProxy(same_target_as)
131 product = (same_target_as.target if self.git else
132 same_target_as.product)
133
134 kwargs = {
135 "information_type": information_type,
136 "owner": owner}
137 if self.git:
138 kwargs["target"] = product
139 return self.factory.makeGitRefs(**kwargs)[0]
140 else:
141 kwargs["product"] = product
142 kwargs["stacked_on"] = stacked_on
143 return self.factory.makeProductBranch(**kwargs)
144
145 def makeBranchMergeProposal(self, source=None, target=None,
146 prerequisite=None, **kwargs):
147 if self.git:
148 return self.factory.makeBranchMergeProposalForGit(
149 target_ref=target, source_ref=source,
150 prerequisite_ref=prerequisite, **kwargs)
151 else:
152 return self.factory.makeBranchMergeProposal(
153 source_branch=source, target_branch=target,
154 prerequisite_branch=prerequisite, **kwargs)
155
156
113class TestBranchMergeProposalInterface(TestCaseWithFactory):157class TestBranchMergeProposalInterface(TestCaseWithFactory):
114 """Ensure that BranchMergeProposal implements its interface."""158 """Ensure that BranchMergeProposal implements its interface."""
115159
@@ -121,26 +165,16 @@ class TestBranchMergeProposalInterface(TestCaseWithFactory):
121 verifyObject(IBranchMergeProposal, bmp)165 verifyObject(IBranchMergeProposal, bmp)
122166
123167
124class TestBranchMergeProposalCanonicalUrl(WithScenarios, TestCaseWithFactory):168class TestBranchMergeProposalCanonicalUrl(
169 WithVCSScenarios, TestCaseWithFactory):
125 """Tests canonical_url for merge proposals."""170 """Tests canonical_url for merge proposals."""
126171
127 layer = DatabaseFunctionalLayer172 layer = DatabaseFunctionalLayer
128173
129 scenarios = [
130 ("bzr", {"git": False}),
131 ("git", {"git": True}),
132 ]
133
134 def _makeBranchMergeProposal(self):
135 if self.git:
136 return self.factory.makeBranchMergeProposalForGit()
137 else:
138 return self.factory.makeBranchMergeProposal()
139
140 def test_BranchMergeProposal_canonical_url_base(self):174 def test_BranchMergeProposal_canonical_url_base(self):
141 # The URL for a merge proposal starts with the parent (source branch175 # The URL for a merge proposal starts with the parent (source branch
142 # or source Git repository).176 # or source Git repository).
143 bmp = self._makeBranchMergeProposal()177 bmp = self.makeBranchMergeProposal()
144 url = canonical_url(bmp)178 url = canonical_url(bmp)
145 parent_url = canonical_url(bmp.parent)179 parent_url = canonical_url(bmp.parent)
146 self.assertTrue(url.startswith(parent_url))180 self.assertTrue(url.startswith(parent_url))
@@ -148,7 +182,7 @@ class TestBranchMergeProposalCanonicalUrl(WithScenarios, TestCaseWithFactory):
148 def test_BranchMergeProposal_canonical_url_rest(self):182 def test_BranchMergeProposal_canonical_url_rest(self):
149 # The rest of the URL for a merge proposal is +merge followed by the183 # The rest of the URL for a merge proposal is +merge followed by the
150 # db id.184 # db id.
151 bmp = self._makeBranchMergeProposal()185 bmp = self.makeBranchMergeProposal()
152 url = canonical_url(bmp)186 url = canonical_url(bmp)
153 parent_url = canonical_url(bmp.parent)187 parent_url = canonical_url(bmp.parent)
154 rest = url[len(parent_url):]188 rest = url[len(parent_url):]
@@ -719,45 +753,14 @@ class TestMergeProposalGetPreviewDiff(TestCaseWithFactory):
719 self.mp_two.getPreviewDiff, self.preview_diff.id)753 self.mp_two.getPreviewDiff, self.preview_diff.id)
720754
721755
722class TestMergeProposalNotification(WithScenarios, TestCaseWithFactory):756class TestMergeProposalNotification(WithVCSScenarios, TestCaseWithFactory):
723 """Test that events are created when merge proposals are manipulated"""757 """Test that events are created when merge proposals are manipulated"""
724758
725 layer = DatabaseFunctionalLayer759 layer = DatabaseFunctionalLayer
726760
727 scenarios = [
728 ("bzr", {"git": False}),
729 ("git", {"git": True}),
730 ]
731
732 def setUp(self):761 def setUp(self):
733 TestCaseWithFactory.setUp(self, user='test@canonical.com')762 TestCaseWithFactory.setUp(self, user='test@canonical.com')
734763
735 def makeBranch(self, same_target_as=None, target=None, **kwargs):
736 kwargs = dict(kwargs)
737 if self.git:
738 if same_target_as is not None:
739 kwargs["target"] = same_target_as.target
740 elif target is not None:
741 kwargs["target"] = target
742 return self.factory.makeGitRefs(**kwargs)[0]
743 else:
744 if same_target_as is not None:
745 kwargs["product"] = same_target_as.product
746 elif target is not None:
747 kwargs["product"] = target
748 return self.factory.makeProductBranch(**kwargs)
749
750 def makeBranchMergeProposal(self, source=None, target=None,
751 prerequisite=None, **kwargs):
752 if self.git:
753 return self.factory.makeBranchMergeProposalForGit(
754 source_ref=source, target_ref=target,
755 prerequisite_ref=prerequisite, **kwargs)
756 else:
757 return self.factory.makeBranchMergeProposal(
758 source_branch=source, target_branch=target,
759 prerequisite_branch=prerequisite, **kwargs)
760
761 def test_notifyOnCreate_needs_review(self):764 def test_notifyOnCreate_needs_review(self):
762 # When a merge proposal is created needing review, the765 # When a merge proposal is created needing review, the
763 # BranchMergeProposalNeedsReviewEvent is raised as well as the usual766 # BranchMergeProposalNeedsReviewEvent is raised as well as the usual
@@ -989,8 +992,8 @@ class TestMergeProposalNotification(WithScenarios, TestCaseWithFactory):
989 # they do not get email about the proposal.992 # they do not get email about the proposal.
990 owner = self.factory.makePerson()993 owner = self.factory.makePerson()
991 product = self.factory.makeProduct()994 product = self.factory.makeProduct()
992 source = self.makeBranch(owner=owner, target=product)995 source = self.makeBranch(owner=owner, product=product)
993 target = self.makeBranch(owner=owner, target=product)996 target = self.makeBranch(owner=owner, product=product)
994 bmp = self.makeBranchMergeProposal(source=source, target=target)997 bmp = self.makeBranchMergeProposal(source=source, target=target)
995 # Subscribe eric to the source branch only.998 # Subscribe eric to the source branch only.
996 eric = self.factory.makePerson()999 eric = self.factory.makePerson()
@@ -1022,40 +1025,15 @@ class TestMergeProposalNotification(WithScenarios, TestCaseWithFactory):
1022 self.assertIn(charlie, recipients)1025 self.assertIn(charlie, recipients)
10231026
10241027
1025class TestMergeProposalWebhooks(WithScenarios, TestCaseWithFactory):1028class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
10261029
1027 layer = DatabaseFunctionalLayer1030 layer = DatabaseFunctionalLayer
10281031
1029 scenarios = [
1030 ("bzr", {"git": False}),
1031 ("git", {"git": True}),
1032 ]
1033
1034 def setUp(self):1032 def setUp(self):
1035 super(TestMergeProposalWebhooks, self).setUp()1033 super(TestMergeProposalWebhooks, self).setUp()
1036 self.useFixture(FeatureFixture(1034 self.useFixture(FeatureFixture(
1037 {BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG: "on"}))1035 {BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG: "on"}))
10381036
1039 def makeBranch(self, same_target_as=None, information_type=None):
1040 # Create the product pillar and its access policy if information
1041 # type is "PROPRIETARY".
1042 if same_target_as is None:
1043 product = self.factory.makeProduct()
1044 if information_type == InformationType.PROPRIETARY:
1045 self.factory.makeAccessPolicy(product)
1046 else:
1047 same_target_as = removeSecurityProxy(same_target_as)
1048 product = (same_target_as.target if self.git else
1049 same_target_as.product)
1050
1051 kwargs = {"information_type": information_type}
1052 if self.git:
1053 kwargs["target"] = product
1054 return self.factory.makeGitRefs(**kwargs)[0]
1055 else:
1056 kwargs["product"] = product
1057 return self.factory.makeProductBranch(**kwargs)
1058
1059 def getWebhookTarget(self, branch):1037 def getWebhookTarget(self, branch):
1060 if self.git:1038 if self.git:
1061 return branch.repository1039 return branch.repository
@@ -1529,15 +1507,10 @@ class TestBranchMergeProposalDeletion(TestCaseWithFactory):
1529 SQLObjectNotFound, BranchMergeProposalJob.get, job_id)1507 SQLObjectNotFound, BranchMergeProposalJob.get, job_id)
15301508
15311509
1532class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):1510class TestBranchMergeProposalBugs(WithVCSScenarios, TestCaseWithFactory):
15331511
1534 layer = DatabaseFunctionalLayer1512 layer = DatabaseFunctionalLayer
15351513
1536 scenarios = [
1537 ("bzr", {"git": False}),
1538 ("git", {"git": True}),
1539 ]
1540
1541 def setUp(self):1514 def setUp(self):
1542 super(TestBranchMergeProposalBugs, self).setUp()1515 super(TestBranchMergeProposalBugs, self).setUp()
1543 self.user = self.factory.makePerson()1516 self.user = self.factory.makePerson()
@@ -1545,15 +1518,9 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1545 if self.git:1518 if self.git:
1546 self.hosting_fixture = self.useFixture(GitHostingFixture())1519 self.hosting_fixture = self.useFixture(GitHostingFixture())
15471520
1548 def _makeBranchMergeProposal(self):
1549 if self.git:
1550 return self.factory.makeBranchMergeProposalForGit()
1551 else:
1552 return self.factory.makeBranchMergeProposal()
1553
1554 def test_bugs(self):1521 def test_bugs(self):
1555 # bugs includes all linked bugs.1522 # bugs includes all linked bugs.
1556 bmp = self._makeBranchMergeProposal()1523 bmp = self.makeBranchMergeProposal()
1557 self.assertEqual([], bmp.bugs)1524 self.assertEqual([], bmp.bugs)
1558 bugs = [self.factory.makeBug() for _ in range(2)]1525 bugs = [self.factory.makeBug() for _ in range(2)]
1559 bmp.linkBug(bugs[0], bmp.registrant)1526 bmp.linkBug(bugs[0], bmp.registrant)
@@ -1568,7 +1535,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1568 def test_related_bugtasks_includes_source_bugtasks(self):1535 def test_related_bugtasks_includes_source_bugtasks(self):
1569 # related_bugtasks includes bugtasks linked to the source branch in1536 # related_bugtasks includes bugtasks linked to the source branch in
1570 # the Bazaar case, or directly to the MP in the Git case.1537 # the Bazaar case, or directly to the MP in the Git case.
1571 bmp = self._makeBranchMergeProposal()1538 bmp = self.makeBranchMergeProposal()
1572 bug = self.factory.makeBug()1539 bug = self.factory.makeBug()
1573 bmp.linkBug(bug, bmp.registrant)1540 bmp.linkBug(bug, bmp.registrant)
1574 self.assertEqual(1541 self.assertEqual(
@@ -1576,7 +1543,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
15761543
1577 def test_related_bugtasks_excludes_private_bugs(self):1544 def test_related_bugtasks_excludes_private_bugs(self):
1578 # related_bugtasks ignores private bugs for non-authorised users.1545 # related_bugtasks ignores private bugs for non-authorised users.
1579 bmp = self._makeBranchMergeProposal()1546 bmp = self.makeBranchMergeProposal()
1580 bug = self.factory.makeBug()1547 bug = self.factory.makeBug()
1581 bmp.linkBug(bug, bmp.registrant)1548 bmp.linkBug(bug, bmp.registrant)
1582 person = self.factory.makePerson()1549 person = self.factory.makePerson()
@@ -1596,7 +1563,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1596 # related_bugtasks ignores bugs linked to the target branch.1563 # related_bugtasks ignores bugs linked to the target branch.
1597 if self.git:1564 if self.git:
1598 self.skipTest("Only relevant for Bazaar.")1565 self.skipTest("Only relevant for Bazaar.")
1599 bmp = self._makeBranchMergeProposal()1566 bmp = self.makeBranchMergeProposal()
1600 bug = self.factory.makeBug()1567 bug = self.factory.makeBug()
1601 bmp.target_branch.linkBug(bug, bmp.registrant)1568 bmp.target_branch.linkBug(bug, bmp.registrant)
1602 self.assertEqual([], list(bmp.getRelatedBugTasks(self.user)))1569 self.assertEqual([], list(bmp.getRelatedBugTasks(self.user)))
@@ -1605,7 +1572,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1605 # related_bugtasks ignores bugs linked to both branches.1572 # related_bugtasks ignores bugs linked to both branches.
1606 if self.git:1573 if self.git:
1607 self.skipTest("Only relevant for Bazaar.")1574 self.skipTest("Only relevant for Bazaar.")
1608 bmp = self._makeBranchMergeProposal()1575 bmp = self.makeBranchMergeProposal()
1609 bug = self.factory.makeBug()1576 bug = self.factory.makeBug()
1610 bmp.source_branch.linkBug(bug, bmp.registrant)1577 bmp.source_branch.linkBug(bug, bmp.registrant)
1611 bmp.target_branch.linkBug(bug, bmp.registrant)1578 bmp.target_branch.linkBug(bug, bmp.registrant)
@@ -1617,7 +1584,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1617 if not self.git:1584 if not self.git:
1618 self.skipTest("Only relevant for Git.")1585 self.skipTest("Only relevant for Git.")
1619 bugs = [self.factory.makeBug() for _ in range(3)]1586 bugs = [self.factory.makeBug() for _ in range(3)]
1620 bmp = self._makeBranchMergeProposal()1587 bmp = self.makeBranchMergeProposal()
1621 self.hosting_fixture.getLog.result = [1588 self.hosting_fixture.getLog.result = [
1622 {1589 {
1623 "sha1": bmp.source_git_commit_sha1,1590 "sha1": bmp.source_git_commit_sha1,
@@ -1664,7 +1631,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1664 # bugs in either the database or the source branch.1631 # bugs in either the database or the source branch.
1665 if not self.git:1632 if not self.git:
1666 self.skipTest("Only relevant for Git.")1633 self.skipTest("Only relevant for Git.")
1667 bmp = self._makeBranchMergeProposal()1634 bmp = self.makeBranchMergeProposal()
1668 self._setUpLog([])1635 self._setUpLog([])
1669 bmp.updateRelatedBugsFromSource()1636 bmp.updateRelatedBugsFromSource()
1670 self.assertEqual([], bmp.bugs)1637 self.assertEqual([], bmp.bugs)
@@ -1675,7 +1642,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1675 if not self.git:1642 if not self.git:
1676 self.skipTest("Only relevant for Git.")1643 self.skipTest("Only relevant for Git.")
1677 bugs = [self.factory.makeBug() for _ in range(3)]1644 bugs = [self.factory.makeBug() for _ in range(3)]
1678 bmp = self._makeBranchMergeProposal()1645 bmp = self.makeBranchMergeProposal()
1679 self._setUpLog([bugs[0]])1646 self._setUpLog([bugs[0]])
1680 bmp.updateRelatedBugsFromSource()1647 bmp.updateRelatedBugsFromSource()
1681 self.assertEqual([bugs[0]], bmp.bugs)1648 self.assertEqual([bugs[0]], bmp.bugs)
@@ -1689,7 +1656,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1689 if not self.git:1656 if not self.git:
1690 self.skipTest("Only relevant for Git.")1657 self.skipTest("Only relevant for Git.")
1691 bug = self.factory.makeBug()1658 bug = self.factory.makeBug()
1692 bmp = self._makeBranchMergeProposal()1659 bmp = self.makeBranchMergeProposal()
1693 self._setUpLog([bug])1660 self._setUpLog([bug])
1694 bmp.updateRelatedBugsFromSource()1661 bmp.updateRelatedBugsFromSource()
1695 self.assertEqual([bug], bmp.bugs)1662 self.assertEqual([bug], bmp.bugs)
@@ -1704,7 +1671,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1704 if not self.git:1671 if not self.git:
1705 self.skipTest("Only relevant for Git.")1672 self.skipTest("Only relevant for Git.")
1706 bug = self.factory.makeBug()1673 bug = self.factory.makeBug()
1707 bmp = self._makeBranchMergeProposal()1674 bmp = self.makeBranchMergeProposal()
1708 self._setUpLog([bug])1675 self._setUpLog([bug])
1709 bmp.updateRelatedBugsFromSource()1676 bmp.updateRelatedBugsFromSource()
1710 self.assertEqual([bug], bmp.bugs)1677 self.assertEqual([bug], bmp.bugs)
@@ -1719,7 +1686,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1719 if not self.git:1686 if not self.git:
1720 self.skipTest("Only relevant for Git.")1687 self.skipTest("Only relevant for Git.")
1721 bug = self.factory.makeBug()1688 bug = self.factory.makeBug()
1722 bmp = self._makeBranchMergeProposal()1689 bmp = self.makeBranchMergeProposal()
1723 bmp.linkBug(bug)1690 bmp.linkBug(bug)
1724 self._setUpLog([])1691 self._setUpLog([])
1725 bmp.updateRelatedBugsFromSource()1692 bmp.updateRelatedBugsFromSource()
@@ -1744,7 +1711,7 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1744 self.skipTest("Only relevant for Git.")1711 self.skipTest("Only relevant for Git.")
1745 self.pushConfig("codehosting", related_bugs_from_source_limit=3)1712 self.pushConfig("codehosting", related_bugs_from_source_limit=3)
1746 bugs = [self.factory.makeBug() for _ in range(5)]1713 bugs = [self.factory.makeBug() for _ in range(5)]
1747 bmp = self._makeBranchMergeProposal()1714 bmp = self.makeBranchMergeProposal()
1748 self._setUpLog([bugs[0]])1715 self._setUpLog([bugs[0]])
1749 bmp.updateRelatedBugsFromSource()1716 bmp.updateRelatedBugsFromSource()
1750 self.assertEqual([bugs[0]], bmp.bugs)1717 self.assertEqual([bugs[0]], bmp.bugs)
@@ -1756,45 +1723,48 @@ class TestBranchMergeProposalBugs(WithScenarios, TestCaseWithFactory):
1756 self.assertEqual("TooManyRelatedBugs", self.oopses[0]["type"])1723 self.assertEqual("TooManyRelatedBugs", self.oopses[0]["type"])
17571724
17581725
1759class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):1726class TestBranchMergeProposalNominateReviewer(
1727 WithVCSScenarios, TestCaseWithFactory):
1760 """Test that the appropriate vote references get created."""1728 """Test that the appropriate vote references get created."""
17611729
1762 layer = DatabaseFunctionalLayer1730 layer = DatabaseFunctionalLayer
17631731
1764 def setUp(self):1732 def setUp(self):
1765 TestCaseWithFactory.setUp(self, user='test@canonical.com')1733 TestCaseWithFactory.setUp(self, user='test@canonical.com')
1734 if self.git:
1735 self.hosting_fixture = self.useFixture(GitHostingFixture())
17661736
1767 def test_notify_on_nominate(self):1737 def test_notify_on_nominate(self):
1768 # Ensure that a notification is emitted on nomination.1738 # Ensure that a notification is emitted on nomination.
1769 merge_proposal = self.factory.makeBranchMergeProposal()1739 merge_proposal = self.makeBranchMergeProposal()
1770 login_person(merge_proposal.source_branch.owner)1740 login_person(merge_proposal.merge_source.owner)
1771 reviewer = self.factory.makePerson()1741 reviewer = self.factory.makePerson()
1772 result, events = self.assertNotifies(1742 result, events = self.assertNotifies(
1773 ReviewerNominatedEvent, False,1743 ReviewerNominatedEvent, False,
1774 merge_proposal.nominateReviewer,1744 merge_proposal.nominateReviewer,
1775 reviewer=reviewer,1745 reviewer=reviewer,
1776 registrant=merge_proposal.source_branch.owner)1746 registrant=merge_proposal.merge_source.owner)
1777 self.assertEqual(result, events[0].object)1747 self.assertEqual(result, events[0].object)
17781748
1779 def test_notify_on_nominate_suppressed_if_requested(self):1749 def test_notify_on_nominate_suppressed_if_requested(self):
1780 # Ensure that a notification is suppressed if notify listeners is set1750 # Ensure that a notification is suppressed if notify listeners is set
1781 # to False.1751 # to False.
1782 merge_proposal = self.factory.makeBranchMergeProposal()1752 merge_proposal = self.makeBranchMergeProposal()
1783 login_person(merge_proposal.source_branch.owner)1753 login_person(merge_proposal.merge_source.owner)
1784 reviewer = self.factory.makePerson()1754 reviewer = self.factory.makePerson()
1785 self.assertNoNotification(1755 self.assertNoNotification(
1786 merge_proposal.nominateReviewer,1756 merge_proposal.nominateReviewer,
1787 reviewer=reviewer,1757 reviewer=reviewer,
1788 registrant=merge_proposal.source_branch.owner,1758 registrant=merge_proposal.merge_source.owner,
1789 _notify_listeners=False)1759 _notify_listeners=False)
17901760
1791 def test_one_initial_votes(self):1761 def test_one_initial_votes(self):
1792 """A new merge proposal has one vote of the default reviewer."""1762 """A new merge proposal has one vote of the default reviewer."""
1793 merge_proposal = self.factory.makeBranchMergeProposal()1763 merge_proposal = self.makeBranchMergeProposal()
1794 self.assertEqual(1, len(list(merge_proposal.votes)))1764 self.assertEqual(1, len(list(merge_proposal.votes)))
1795 [vote] = list(merge_proposal.votes)1765 [vote] = list(merge_proposal.votes)
1796 self.assertEqual(1766 self.assertEqual(
1797 merge_proposal.target_branch.owner, vote.reviewer)1767 merge_proposal.merge_target.owner, vote.reviewer)
17981768
1799 def makeProposalWithReviewer(self, reviewer=None, review_type=None,1769 def makeProposalWithReviewer(self, reviewer=None, review_type=None,
1800 registrant=None, **kwargs):1770 registrant=None, **kwargs):
@@ -1807,8 +1777,9 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
1807 if registrant is None:1777 if registrant is None:
1808 registrant = self.factory.makePerson()1778 registrant = self.factory.makePerson()
1809 merge_proposal = make_merge_proposal_without_reviewers(1779 merge_proposal = make_merge_proposal_without_reviewers(
1810 factory=self.factory, registrant=registrant, **kwargs)1780 factory=self.factory, for_git=self.git, registrant=registrant,
1811 login_person(merge_proposal.source_branch.owner)1781 **kwargs)
1782 login_person(merge_proposal.merge_source.owner)
1812 merge_proposal.nominateReviewer(1783 merge_proposal.nominateReviewer(
1813 reviewer=reviewer, registrant=registrant, review_type=review_type)1784 reviewer=reviewer, registrant=registrant, review_type=review_type)
1814 return merge_proposal, reviewer1785 return merge_proposal, reviewer
@@ -1914,15 +1885,15 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
19141885
1915 def test_nominate_updates_reference(self):1886 def test_nominate_updates_reference(self):
1916 """The existing reference is updated on re-nomination."""1887 """The existing reference is updated on re-nomination."""
1917 merge_proposal = self.factory.makeBranchMergeProposal()1888 merge_proposal = self.makeBranchMergeProposal()
1918 login_person(merge_proposal.source_branch.owner)1889 login_person(merge_proposal.merge_source.owner)
1919 reviewer = self.factory.makePerson()1890 reviewer = self.factory.makePerson()
1920 reference = merge_proposal.nominateReviewer(1891 reference = merge_proposal.nominateReviewer(
1921 reviewer=reviewer, registrant=merge_proposal.source_branch.owner,1892 reviewer=reviewer, registrant=merge_proposal.merge_source.owner,
1922 review_type='General')1893 review_type='General')
1923 self.assertEqual('general', reference.review_type)1894 self.assertEqual('general', reference.review_type)
1924 merge_proposal.nominateReviewer(1895 merge_proposal.nominateReviewer(
1925 reviewer=reviewer, registrant=merge_proposal.source_branch.owner,1896 reviewer=reviewer, registrant=merge_proposal.merge_source.owner,
1926 review_type='Specific')1897 review_type='Specific')
1927 # Note we're using the reference from the first call1898 # Note we're using the reference from the first call
1928 self.assertEqual('specific', reference.review_type)1899 self.assertEqual('specific', reference.review_type)
@@ -1939,31 +1910,37 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
1939 CodeReviewNotificationLevel.FULL, sub.review_level)1910 CodeReviewNotificationLevel.FULL, sub.review_level)
1940 # The reviewer can see the branch.1911 # The reviewer can see the branch.
1941 self.assertTrue(branch.visibleByUser(reviewer))1912 self.assertTrue(branch.visibleByUser(reviewer))
1942 if branch.stacked_on is not None:1913 if IBranch.providedBy(branch) and branch.stacked_on is not None:
1943 self._check_mp_branch_visibility(branch.stacked_on, reviewer)1914 self._check_mp_branch_visibility(branch.stacked_on, reviewer)
19441915
1945 def _test_nominate_grants_visibility(self, reviewer):1916 def _test_nominate_grants_visibility(self, reviewer):
1946 """Nominated reviewers can see the source and target branches."""1917 """Nominated reviewers can see the source and target branches."""
1947 owner = self.factory.makePerson()1918 owner = self.factory.makePerson()
1948 product = self.factory.makeProduct()1919 product = self.factory.makeProduct()
1949 # We make a source branch stacked on a private one.1920 # For bzr, we make a source branch stacked on a private one.
1950 base_branch = self.factory.makeBranch(1921 # For git, we make the gitref itself private.
1922 if self.git:
1923 source_branch = self.makeBranch(
1924 product=product, owner=owner,
1925 information_type=InformationType.USERDATA)
1926 else:
1927 base_branch = self.makeBranch(
1928 owner=owner, product=product,
1929 information_type=InformationType.USERDATA)
1930 source_branch = self.makeBranch(
1931 stacked_on=base_branch, product=product, owner=owner)
1932 target_branch = self.makeBranch(
1951 owner=owner, product=product,1933 owner=owner, product=product,
1952 information_type=InformationType.USERDATA)1934 information_type=InformationType.USERDATA)
1953 source_branch = self.factory.makeBranch(
1954 stacked_on=base_branch, product=product, owner=owner)
1955 target_branch = self.factory.makeBranch(owner=owner, product=product)
1956 login_person(owner)1935 login_person(owner)
1957 merge_proposal = self.factory.makeBranchMergeProposal(1936 merge_proposal = self.makeBranchMergeProposal(
1958 source_branch=source_branch,1937 source=source_branch, target=target_branch)
1959 target_branch=target_branch)
1960 target_branch.setPrivate(True, owner)
1961 # The reviewer can't see the source or target branches.1938 # The reviewer can't see the source or target branches.
1962 self.assertFalse(source_branch.visibleByUser(reviewer))1939 self.assertFalse(source_branch.visibleByUser(reviewer))
1963 self.assertFalse(target_branch.visibleByUser(reviewer))1940 self.assertFalse(target_branch.visibleByUser(reviewer))
1964 merge_proposal.nominateReviewer(1941 merge_proposal.nominateReviewer(
1965 reviewer=reviewer,1942 reviewer=reviewer,
1966 registrant=merge_proposal.source_branch.owner)1943 registrant=merge_proposal.merge_source.owner)
1967 for branch in [source_branch, target_branch]:1944 for branch in [source_branch, target_branch]:
1968 self._check_mp_branch_visibility(branch, reviewer)1945 self._check_mp_branch_visibility(branch, reviewer)
19691946
@@ -1987,7 +1964,7 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
1987 def test_comment_with_vote_creates_reference(self):1964 def test_comment_with_vote_creates_reference(self):
1988 """A comment with a vote creates a vote reference."""1965 """A comment with a vote creates a vote reference."""
1989 reviewer = self.factory.makePerson()1966 reviewer = self.factory.makePerson()
1990 merge_proposal = self.factory.makeBranchMergeProposal(1967 merge_proposal = self.makeBranchMergeProposal(
1991 reviewer=reviewer, registrant=reviewer)1968 reviewer=reviewer, registrant=reviewer)
1992 comment = merge_proposal.createComment(1969 comment = merge_proposal.createComment(
1993 reviewer, 'Message subject', 'Message content',1970 reviewer, 'Message subject', 'Message content',
@@ -1998,7 +1975,8 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
1998 def test_comment_without_a_vote_does_not_create_reference(self):1975 def test_comment_without_a_vote_does_not_create_reference(self):
1999 """A comment with a vote creates a vote reference."""1976 """A comment with a vote creates a vote reference."""
2000 reviewer = self.factory.makePerson()1977 reviewer = self.factory.makePerson()
2001 merge_proposal = make_merge_proposal_without_reviewers(self.factory)1978 merge_proposal = make_merge_proposal_without_reviewers(
1979 self.factory, for_git=self.git)
2002 merge_proposal.createComment(1980 merge_proposal.createComment(
2003 reviewer, 'Message subject', 'Message content')1981 reviewer, 'Message subject', 'Message content')
2004 self.assertEqual([], list(merge_proposal.votes))1982 self.assertEqual([], list(merge_proposal.votes))
@@ -2006,7 +1984,7 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
2006 def test_second_vote_by_person_just_alters_reference(self):1984 def test_second_vote_by_person_just_alters_reference(self):
2007 """A second vote changes the comment reference only."""1985 """A second vote changes the comment reference only."""
2008 reviewer = self.factory.makePerson()1986 reviewer = self.factory.makePerson()
2009 merge_proposal = self.factory.makeBranchMergeProposal(1987 merge_proposal = self.makeBranchMergeProposal(
2010 reviewer=reviewer, registrant=reviewer)1988 reviewer=reviewer, registrant=reviewer)
2011 merge_proposal.createComment(1989 merge_proposal.createComment(
2012 reviewer, 'Message subject', 'Message content',1990 reviewer, 'Message subject', 'Message content',
@@ -2022,7 +2000,7 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
2022 reviewer = self.factory.makePerson()2000 reviewer = self.factory.makePerson()
2023 merge_proposal, ignore = self.makeProposalWithReviewer(2001 merge_proposal, ignore = self.makeProposalWithReviewer(
2024 reviewer=reviewer, review_type='general')2002 reviewer=reviewer, review_type='general')
2025 login(merge_proposal.source_branch.owner.preferredemail.email)2003 login(merge_proposal.merge_source.owner.preferredemail.email)
2026 comment = merge_proposal.createComment(2004 comment = merge_proposal.createComment(
2027 reviewer, 'Message subject', 'Message content',2005 reviewer, 'Message subject', 'Message content',
2028 vote=CodeReviewVote.APPROVE, review_type='general')2006 vote=CodeReviewVote.APPROVE, review_type='general')
@@ -2042,7 +2020,7 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
2042 team = self.factory.makeTeam(owner=reviewer)2020 team = self.factory.makeTeam(owner=reviewer)
2043 merge_proposal, ignore = self.makeProposalWithReviewer(2021 merge_proposal, ignore = self.makeProposalWithReviewer(
2044 reviewer=team, review_type='general')2022 reviewer=team, review_type='general')
2045 login(merge_proposal.source_branch.owner.preferredemail.email)2023 login(merge_proposal.merge_source.owner.preferredemail.email)
2046 [vote] = list(merge_proposal.votes)2024 [vote] = list(merge_proposal.votes)
2047 self.assertEqual(team, vote.reviewer)2025 self.assertEqual(team, vote.reviewer)
2048 comment = merge_proposal.createComment(2026 comment = merge_proposal.createComment(
@@ -2059,8 +2037,8 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
2059 # one.2037 # one.
2060 reviewer = self.factory.makePerson()2038 reviewer = self.factory.makePerson()
2061 team = self.factory.makeTeam(owner=reviewer)2039 team = self.factory.makeTeam(owner=reviewer)
2062 merge_proposal = self.factory.makeBranchMergeProposal(reviewer=team)2040 merge_proposal = self.makeBranchMergeProposal(reviewer=team)
2063 login(merge_proposal.source_branch.owner.preferredemail.email)2041 login(merge_proposal.merge_source.owner.preferredemail.email)
2064 [vote] = list(merge_proposal.votes)2042 [vote] = list(merge_proposal.votes)
2065 self.assertEqual(team, vote.reviewer)2043 self.assertEqual(team, vote.reviewer)
2066 comment = merge_proposal.createComment(2044 comment = merge_proposal.createComment(
@@ -2078,8 +2056,8 @@ class TestBranchMergeProposalNominateReviewer(TestCaseWithFactory):
2078 merge_proposal_1, reviewer_1 = self.makeProposalWithReviewer(2056 merge_proposal_1, reviewer_1 = self.makeProposalWithReviewer(
2079 set_state=BranchMergeProposalStatus.MERGED)2057 set_state=BranchMergeProposalStatus.MERGED)
2080 merge_proposal_2, _ = self.makeProposalWithReviewer(2058 merge_proposal_2, _ = self.makeProposalWithReviewer(
2081 target_branch=merge_proposal_1.target_branch,2059 target=merge_proposal_1.merge_target,
2082 source_branch=merge_proposal_1.source_branch)2060 source=merge_proposal_1.merge_source)
2083 merge_proposal_2.nominateReviewer(2061 merge_proposal_2.nominateReviewer(
2084 reviewer=reviewer_1, registrant=merge_proposal_2.registrant)2062 reviewer=reviewer_1, registrant=merge_proposal_2.registrant)
2085 votes_1 = list(merge_proposal_1.votes)2063 votes_1 = list(merge_proposal_1.votes)
diff --git a/lib/lp/code/tests/helpers.py b/lib/lp/code/tests/helpers.py
index 46f12ed..0e784f0 100644
--- a/lib/lp/code/tests/helpers.py
+++ b/lib/lp/code/tests/helpers.py
@@ -1,4 +1,4 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Helper functions for code testing live here."""4"""Helper functions for code testing live here."""
@@ -49,8 +49,8 @@ from lp.code.model.seriessourcepackagebranch import (
49from lp.registry.interfaces.pocket import PackagePublishingPocket49from lp.registry.interfaces.pocket import PackagePublishingPocket
50from lp.registry.interfaces.series import SeriesStatus50from lp.registry.interfaces.series import SeriesStatus
51from lp.services.database.sqlbase import cursor51from lp.services.database.sqlbase import cursor
52from lp.services.propertycache import get_property_cache
53from lp.services.memcache.testing import MemcacheFixture52from lp.services.memcache.testing import MemcacheFixture
53from lp.services.propertycache import get_property_cache
54from lp.testing import (54from lp.testing import (
55 run_with_login,55 run_with_login,
56 time_counter,56 time_counter,
@@ -271,9 +271,18 @@ def recipe_parser_newest_version(version):
271 RecipeParser.NEWEST_VERSION = old_version271 RecipeParser.NEWEST_VERSION = old_version
272272
273273
274def make_merge_proposal_without_reviewers(factory, **kwargs):274def make_merge_proposal_without_reviewers(
275 factory, for_git=False, source=None, target=None, **kwargs):
275 """Make a merge proposal and strip of any review votes."""276 """Make a merge proposal and strip of any review votes."""
276 proposal = factory.makeBranchMergeProposal(**kwargs)277 kwargs = dict(kwargs)
278 if for_git:
279 kwargs["source_ref"] = source
280 kwargs["target_ref"] = target
281 proposal = factory.makeBranchMergeProposalForGit(**kwargs)
282 else:
283 kwargs["source_branch"] = source
284 kwargs["target_branch"] = target
285 proposal = factory.makeBranchMergeProposal(**kwargs)
277 for vote in proposal.votes:286 for vote in proposal.votes:
278 removeSecurityProxy(vote).destroySelf()287 removeSecurityProxy(vote).destroySelf()
279 del get_property_cache(proposal).votes288 del get_property_cache(proposal).votes