launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/precache-gitrepository-branch-queries into lp:launchpad.

Commit message:
Fix tests broken by the change to a cachedproperty

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1790047 in Launchpad itself: "GitRepository:++repository-pending-merges shows too many MPs and runs too many queries"
  https://bugs.launchpad.net/launchpad/+bug/1790047

For more details, see:
https://code.launchpad.net/~twom/launchpad/precache-gitrepository-branch-queries/+merge/354470

BranchMergeProposal.votes changed to a cachedproperty, this changes tests to clear that cache once the BMP is loaded and altered.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/precache-gitrepository-branch-queries into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2018-06-22 10:01:34 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2018-09-07 11:22:58 +0000
@@ -568,7 +568,7 @@
         view = create_view(branch, '+index')
         with StormStatementRecorder() as recorder:
             view.landing_candidates
-        self.assertThat(recorder, HasQueryCount(Equals(13)))
+        self.assertThat(recorder, HasQueryCount(Equals(14)))
 
     def test_query_count_landing_targets(self):
         product = self.factory.makeProduct()
@@ -586,7 +586,7 @@
         view = create_view(branch, '+index')
         with StormStatementRecorder() as recorder:
             view.landing_targets
-        self.assertThat(recorder, HasQueryCount(Equals(12)))
+        self.assertThat(recorder, HasQueryCount(Equals(13)))
 
     def test_query_count_subscriber_content(self):
         branch = self.factory.makeBranch()

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2018-06-21 17:26:43 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2018-09-07 11:22:58 +0000
@@ -92,6 +92,7 @@
 from lp.services.features.testing import FeatureFixture
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.messages.model.message import MessageSet
+from lp.services.propertycache import clear_property_cache
 from lp.services.timeout import TimeoutError
 from lp.services.webapp import canonical_url
 from lp.services.webapp.interfaces import BrowserNotificationLevel
@@ -308,6 +309,7 @@
         # nominating reviewers.
         TestCaseWithFactory.setUp(self, user="admin@xxxxxxxxxxxxx")
         self.bmp = make_merge_proposal_without_reviewers(self.factory)
+        clear_property_cache(self.bmp)
         self.date_generator = time_counter(delta=timedelta(days=1))
 
     def _createComment(self, reviewer, vote):
@@ -453,6 +455,7 @@
         self._createComment(bob, CodeReviewVote.ABSTAIN)
         self._createComment(charles, CodeReviewVote.DISAPPROVE)
 
+        clear_property_cache(self.bmp)
         view = BranchMergeProposalVoteView(self.bmp, LaunchpadTestRequest())
 
         self.assertEqual(
@@ -468,6 +471,7 @@
         self._createComment(bob, CodeReviewVote.APPROVE)
         self._createComment(albert, CodeReviewVote.APPROVE)
 
+        clear_property_cache(self.bmp)
         view = BranchMergeProposalVoteView(self.bmp, LaunchpadTestRequest())
 
         self.assertEqual(

=== modified file 'lib/lp/code/browser/tests/test_gitref.py'
--- lib/lp/code/browser/tests/test_gitref.py	2018-08-24 16:52:27 +0000
+++ lib/lp/code/browser/tests/test_gitref.py	2018-09-07 11:22:58 +0000
@@ -295,7 +295,7 @@
         view = create_view(ref, '+index')
         with StormStatementRecorder() as recorder:
             view.landing_candidates
-        self.assertThat(recorder, HasQueryCount(Equals(11)))
+        self.assertThat(recorder, HasQueryCount(Equals(12)))
 
     def test_query_count_landing_targets(self):
         project = self.factory.makeProduct()
@@ -311,4 +311,4 @@
         view = create_view(ref, '+index')
         with StormStatementRecorder() as recorder:
             view.landing_targets
-        self.assertThat(recorder, HasQueryCount(Equals(11)))
+        self.assertThat(recorder, HasQueryCount(Equals(12)))

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2018-04-25 12:01:33 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2018-09-07 11:22:58 +0000
@@ -85,6 +85,7 @@
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
+from lp.services.propertycache import clear_property_cache
 from lp.services.webapp import canonical_url
 from lp.services.xref.interfaces import IXRefSet
 from lp.testing import (
@@ -1782,6 +1783,7 @@
         # A new vote reference is created when a reviewer is nominated.
         merge_proposal, reviewer = self.makeProposalWithReviewer(
             review_type='general')
+        clear_property_cache(merge_proposal)
         self.assertOneReviewPending(merge_proposal, reviewer, 'general')
 
     def test_nominate_with_None_review_type(self):
@@ -1789,6 +1791,7 @@
         # with a review_type of None.
         merge_proposal, reviewer = self.makeProposalWithReviewer(
             review_type=None)
+        clear_property_cache(merge_proposal)
         self.assertOneReviewPending(merge_proposal, reviewer, None)
 
     def test_nominate_with_whitespace_review_type(self):
@@ -1797,12 +1800,15 @@
         # None.
         merge_proposal, reviewer = self.makeProposalWithReviewer(
             review_type='')
+        clear_property_cache(merge_proposal)
         self.assertOneReviewPending(merge_proposal, reviewer, None)
         merge_proposal, reviewer = self.makeProposalWithReviewer(
             review_type='    ')
+        clear_property_cache(merge_proposal)
         self.assertOneReviewPending(merge_proposal, reviewer, None)
         merge_proposal, reviewer = self.makeProposalWithReviewer(
             review_type='\t')
+        clear_property_cache(merge_proposal)
         self.assertOneReviewPending(merge_proposal, reviewer, None)
 
     def test_nominate_multiple_with_different_types(self):
@@ -1817,6 +1823,7 @@
             reviewer=review_team,
             registrant=merge_proposal.registrant,
             review_type='general-2')
+        clear_property_cache(merge_proposal)
 
         votes = list(merge_proposal.votes)
         self.assertEqual(
@@ -1833,6 +1840,7 @@
             reviewer=review_team,
             registrant=merge_proposal.registrant,
             review_type='general')
+        clear_property_cache(merge_proposal)
 
         votes = list(merge_proposal.votes)
         self.assertEqual(
@@ -1849,6 +1857,7 @@
             reviewer=review_team,
             registrant=merge_proposal.registrant,
             review_type=None)
+        clear_property_cache(merge_proposal)
 
         votes = list(merge_proposal.votes)
         self.assertEqual(
@@ -1863,6 +1872,7 @@
         reference = merge_proposal.nominateReviewer(
             reviewer=reviewer, registrant=merge_proposal.source_branch.owner,
             review_type='General')
+        clear_property_cache(merge_proposal)
         self.assertEqual('general', reference.review_type)
         merge_proposal.nominateReviewer(
             reviewer=reviewer, registrant=merge_proposal.source_branch.owner,
@@ -1932,6 +1942,7 @@
         reviewer = self.factory.makePerson()
         merge_proposal = self.factory.makeBranchMergeProposal(
             reviewer=reviewer, registrant=reviewer)
+        clear_property_cache(merge_proposal)
         comment = merge_proposal.createComment(
             reviewer, 'Message subject', 'Message content',
             vote=CodeReviewVote.APPROVE)
@@ -1944,6 +1955,7 @@
         merge_proposal = make_merge_proposal_without_reviewers(self.factory)
         merge_proposal.createComment(
             reviewer, 'Message subject', 'Message content')
+        clear_property_cache(merge_proposal)
         self.assertEqual([], list(merge_proposal.votes))
 
     def test_second_vote_by_person_just_alters_reference(self):
@@ -1965,6 +1977,7 @@
         reviewer = self.factory.makePerson()
         merge_proposal, ignore = self.makeProposalWithReviewer(
             reviewer=reviewer, review_type='general')
+        clear_property_cache(merge_proposal)
         login(merge_proposal.source_branch.owner.preferredemail.email)
         comment = merge_proposal.createComment(
             reviewer, 'Message subject', 'Message content',
@@ -1985,6 +1998,7 @@
         team = self.factory.makeTeam(owner=reviewer)
         merge_proposal, ignore = self.makeProposalWithReviewer(
             reviewer=team, review_type='general')
+        clear_property_cache(merge_proposal)
         login(merge_proposal.source_branch.owner.preferredemail.email)
         [vote] = list(merge_proposal.votes)
         self.assertEqual(team, vote.reviewer)

=== modified file 'lib/lp/code/model/tests/test_codereviewvote.py'
--- lib/lp/code/model/tests/test_codereviewvote.py	2017-10-04 01:53:48 +0000
+++ lib/lp/code/model/tests/test_codereviewvote.py	2018-09-07 11:22:58 +0000
@@ -14,6 +14,7 @@
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.code.tests.helpers import make_merge_proposal_without_reviewers
 from lp.services.database.constants import UTC_NOW
+from lp.services.propertycache import clear_property_cache
 from lp.testing import (
     login_person,
     TestCaseWithFactory,
@@ -28,6 +29,7 @@
     def test_create_vote(self):
         """CodeReviewVotes can be created"""
         merge_proposal = make_merge_proposal_without_reviewers(self.factory)
+        clear_property_cache(merge_proposal)
         reviewer = self.factory.makePerson()
         login_person(merge_proposal.registrant)
         vote = merge_proposal.nominateReviewer(
@@ -136,6 +138,7 @@
         # A pending review can be deleted by the person requesting the review.
         reviewer = self.factory.makePerson()
         bmp = make_merge_proposal_without_reviewers(self.factory)
+        clear_property_cache(bmp)
         login_person(bmp.registrant)
         review = bmp.nominateReviewer(
             reviewer=reviewer, registrant=bmp.registrant)
@@ -146,6 +149,7 @@
         # A pending review can be deleted by the person requesting the review.
         reviewer = self.factory.makePerson()
         bmp = make_merge_proposal_without_reviewers(self.factory)
+        clear_property_cache(bmp)
         login_person(bmp.registrant)
         review = bmp.nominateReviewer(
             reviewer=reviewer, registrant=bmp.registrant)
@@ -162,6 +166,7 @@
             reviewer=review_team, registrant=bmp.registrant)
         login_person(review_team.teamowner)
         review.delete()
+        clear_property_cache(bmp)
         self.assertEqual([], list(bmp.votes))
 
     def test_delete_pending_by_target_branch_owner(self):
@@ -169,6 +174,7 @@
         # the target branch.
         reviewer = self.factory.makePerson()
         bmp = make_merge_proposal_without_reviewers(self.factory)
+        clear_property_cache(bmp)
         login_person(bmp.registrant)
         review = bmp.nominateReviewer(
             reviewer=reviewer, registrant=bmp.registrant)