Launchpad itself

Merge lp:~james-w/launchpad/fix-getRequestedReviews into lp:launchpad

  1. fix-getRequestedReviews
  2. Merge into devel
Proposed by James Westby
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~james-w/launchpad/fix-getRequestedReviews
Merge into: lp:launchpad
Diff against target: 52 lines (+27/-3)
2 files modified
lib/lp/code/model/hasbranches.py (+1/-1)
lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+26/-2)
To merge this branch: bzr merge lp:~james-w/launchpad/fix-getRequestedReviews
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Aaron Bentley (community) Needs Fixing
Review via email: mp+20378@code.launchpad.net

Commit message

getRequestedReviews no longer returns the merge proposals owned by the person in question.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Hi,

This fixes a bug with my getRequestedReviews implementation, which
was basically that I fail at reading docstrings.

This switches it to use the method that matches what getRequestedReviews
says that it will do, and modifies the test so that it should set up
an environment that will distinguish the two.

It is however untested.

Thanks,

James

Revision history for this message
Aaron Bentley (abentley) wrote :
Download full text (5.9 KiB)

> It is however untested.

The test fails:

abentley@lumpy:~/launchpad/fix-getRequestedReviews$ bin/test -t xx-branchmergeproposal.txt
Running canonical.testing.layers.PageTestLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.002 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 0.348 seconds.
  Set up canonical.testing.layers.LibrarianLayer in 7.496 seconds.
  Set up canonical.testing.layers.MemcachedLayer in 0.115 seconds.
  Set up canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 4.550 seconds.
  Set up canonical.testing.layers.GoogleServiceLayer in 2.321 seconds.
  Set up canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Set up canonical.testing.layers.PageTestLayer in 0.001 seconds.

Failure in test lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt
Failed doctest test for xx-branchmergeproposal.txt
  File "lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt", line 0

----------------------------------------------------------------------
File "lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt", line 474, in xx-branchmergeproposal.txt
Failed example:
    proposals = webservice.named_get('/~target', 'getRequestedReviews'
        ).jsonBody()
Exception raised:
    Traceback (most recent call last):
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/zope.testing-3.8.1-py2.5.egg/zope/testing/doctest.py", line 1361, in __run
        compileflags, 1) in test.globs
      File "<doctest xx-branchmergeproposal.txt[line 474, example 119]>", line 1, in <module>
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 272, in named_get
        api_version=api_version)
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 232, in get
        api_version=api_version)
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 221, in __call__
        self.addHeadersTo(full_url, full_headers) # a hook
      File "/home/abentley/launchpad/fix-getRequestedReviews/lib/canonical/launchpad/testing/pages.py", line 139, in addHeadersTo
        self.consumer, self.access_token, http_url = full_url,
      File "/home/abentley/launchpad/fix-getRequestedReviews/lib/contrib/oauth.py", line 216, in from_consumer_and_token
        'oauth_consumer_key': oauth_consumer.key,
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/properties.py", line 60, in __get__
        return obj_info.variables[column].get()
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/store.py", line 879, in _resolve_lazy_value
        self.flush()
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/store.py", line 486, in flush
        self._flush_one(obj_info...

Read more...

review: Needs Fixing
Revision history for this message
James Westby (james-w) wrote :

Hi,

The test now passes.

Thanks,

James

[ Code changes < 5 minutes, waiting for test set up + tear down ~ 8 minutes,
  getting all the pieces in place to run the testsuite after not having done
  so for a few weeks ~ 80 minutes ]

Revision history for this message
Graham Binns (gmb) wrote :

This code looks good now the test passes.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/hasbranches.py'
2--- lib/lp/code/model/hasbranches.py 2010-02-08 14:37:50 +0000
3+++ lib/lp/code/model/hasbranches.py 2010-03-02 01:06:25 +0000
4@@ -59,5 +59,5 @@
5
6 visible_branches = getUtility(IAllBranches).visibleByUser(
7 visible_by_user)
8- proposals = visible_branches.getMergeProposalsForPerson(self, status)
9+ proposals = visible_branches.getMergeProposalsForReviewer(self, status)
10 return proposals
11
12=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
13--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2010-02-26 00:30:09 +0000
14+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2010-03-02 01:06:25 +0000
15@@ -464,12 +464,36 @@
16
17 >>> login('admin@canonical.com')
18 >>> from lp.code.enums import BranchMergeProposalStatus
19- >>> fixit_proposal.requestReview()
20+
21+First we create a review owned by someone else and requested of 'target'
22+which is the one we want the method to return.
23+
24+ >>> source_branch = factory.makeBranch(owner=branch_owner,
25+ ... product=blob, name="foo")
26+ >>> target_branch = factory.makeBranch(owner=target_owner,
27+ ... product=blob, name="bar")
28+ >>> proposal = factory.makeBranchMergeProposal(
29+ ... target_branch=target_branch,
30+ ... product=blob, set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
31+ ... registrant=branch_owner, source_branch=source_branch)
32+ >>> proposal.nominateReviewer(target_owner, branch_owner)
33+ <CodeReviewVoteReference at ...>
34+
35+And then we propose a merge the other way, so that the owner is target,
36+but they have not been asked to review, meaning that the method shouldn't
37+return this review.
38+
39+ >>> proposal = factory.makeBranchMergeProposal(
40+ ... target_branch=source_branch,
41+ ... product=blob, set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
42+ ... registrant=target_owner, source_branch=target_branch)
43+ >>> proposal.nominateReviewer(branch_owner, target_owner)
44+ <CodeReviewVoteReference at ...>
45 >>> logout()
46
47 >>> proposals = webservice.named_get('/~target', 'getRequestedReviews'
48 ... ).jsonBody()
49 >>> for proposal in proposals['entries']:
50 ... print_proposal(proposal)
51- http://.../~source/fooix/fix-it/+merge/2 - Needs review
52+ http://.../~source/blob/foo/+merge/4 - Needs review
53