Merge lp:~stevenk/launchpad/preview-diff-none-type into lp:launchpad

[Steve Kowalik]

Update lazr.restful to 0.19.8, and add a test for fetching a NULL preview_diff of an BMP over the API.

IDiff.diffstat is exported as Dict(), so lazr.restful attempts to unmarshall it as so. This would be fine, except when the column in the DB is NULL and the method returns None. Update to lazr.restful 0.19.8 solving the failure.

I have clawed back the LoC by some refactoring of the tests and removing the now unneeded pylint garbage.

[Ian Booth (wallyworld)]

Looks great

review: Approve

=== modified file 'lib/lp/code/interfaces/diff.py'
--- lib/lp/code/interfaces/diff.py	2011-12-24 16:54:44 +0000
+++ lib/lp/code/interfaces/diff.py	2012-10-02 01:43:21 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0211,E0213
-
 """Interfaces including and related to IDiff."""
 
 __metaclass__ = type

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2012-09-19 01:19:35 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2012-10-02 01:43:21 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=F0401
-
 """Tests for BranchMergeProposals."""
 
 __metaclass__ = type
@@ -1521,13 +1519,11 @@
         login_person(merge_proposal.source_branch.owner)
         reviewer = self.factory.makePerson()
         reference = merge_proposal.nominateReviewer(
-            reviewer=reviewer,
-            registrant=merge_proposal.source_branch.owner,
+            reviewer=reviewer, registrant=merge_proposal.source_branch.owner,
             review_type='General')
         self.assertEqual('general', reference.review_type)
         merge_proposal.nominateReviewer(
-            reviewer=reviewer,
-            registrant=merge_proposal.source_branch.owner,
+            reviewer=reviewer, registrant=merge_proposal.source_branch.owner,
             review_type='Specific')
         # Note we're using the reference from the first call
         self.assertEqual('specific', reference.review_type)
@@ -1539,11 +1535,9 @@
             BranchSubscriptionNotificationLevel.NOEMAIL,
             sub.notification_level)
         self.assertEqual(
-            BranchSubscriptionDiffSize.NODIFF,
-            sub.max_diff_lines)
+            BranchSubscriptionDiffSize.NODIFF, sub.max_diff_lines)
         self.assertEqual(
-            CodeReviewNotificationLevel.FULL,
-            sub.review_level)
+            CodeReviewNotificationLevel.FULL, sub.review_level)
         # The reviewer can see the branch.
         self.assertTrue(branch.visibleByUser(reviewer))
         if branch.stacked_on is not None:
@@ -1585,6 +1579,14 @@
             membership_policy=TeamMembershipPolicy.MODERATED)
         self._test_nominate_grants_visibility(reviewer)
 
+    def _assertVoteReference(self, votes, reviewer, comment):
+        self.assertEqual(1, len(votes))
+        vote_reference = votes[0]
+        self.assertEqual(reviewer, vote_reference.reviewer)
+        self.assertEqual(reviewer, vote_reference.registrant)
+        self.assertIsNone(vote_reference.review_type)
+        self.assertEqual(comment, vote_reference.comment)
+
     def test_comment_with_vote_creates_reference(self):
         """A comment with a vote creates a vote reference."""
         reviewer = self.factory.makePerson()
@@ -1594,12 +1596,7 @@
             reviewer, 'Message subject', 'Message content',
             vote=CodeReviewVote.APPROVE)
         votes = list(merge_proposal.votes)
-        self.assertEqual(1, len(votes))
-        vote_reference = votes[0]
-        self.assertEqual(reviewer, vote_reference.reviewer)
-        self.assertEqual(reviewer, vote_reference.registrant)
-        self.assertTrue(vote_reference.review_type is None)
-        self.assertEqual(comment, vote_reference.comment)
+        self._assertVoteReference(votes, reviewer, comment)
 
     def test_comment_without_a_vote_does_not_create_reference(self):
         """A comment with a vote creates a vote reference."""
@@ -1621,12 +1618,7 @@
             reviewer, 'Message subject', 'Message content',
             vote=CodeReviewVote.APPROVE)
         votes = list(merge_proposal.votes)
-        self.assertEqual(1, len(votes))
-        vote_reference = votes[0]
-        self.assertEqual(reviewer, vote_reference.reviewer)
-        self.assertEqual(reviewer, vote_reference.registrant)
-        self.assertTrue(vote_reference.review_type is None)
-        self.assertEqual(comment2, vote_reference.comment)
+        self._assertVoteReference(votes, reviewer, comment2)
 
     def test_vote_by_nominated_reuses_reference(self):
         """A comment with a vote for a nominated reviewer alters reference."""
@@ -2086,3 +2078,11 @@
             BadRequest,
             '(.|\n)*Invalid state transition for merge proposal(.|\n)*'):
             ws_bmp.setStatus(status='Approved')
+
+    def test_previewdiff_with_null_diffstat(self):
+        # A previewdiff with an empty diffstat doesn't crash when fetched.
+        previewdiff = self.factory.makePreviewDiff()
+        previewdiff.diff.diffstat = None
+        user = previewdiff.branch_merge_proposal.target_branch.owner
+        ws_previewdiff = self.wsObject(previewdiff, user=user)
+        self.assertIsNone(ws_previewdiff.diffstat)

=== modified file 'versions.cfg'
--- versions.cfg	2012-09-26 07:59:35 +0000
+++ versions.cfg	2012-10-02 01:43:21 +0000
@@ -49,7 +49,7 @@
 lazr.enum = 1.1.3
 lazr.jobrunner = 0.10
 lazr.lifecycle = 1.1
-lazr.restful = 0.19.7
+lazr.restful = 0.19.8
 lazr.restfulclient = 0.13.1
 lazr.smtptest = 1.3
 lazr.testing = 0.1.1