launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/remove-resolved-packagediffs-bug-764263 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #764263 in Launchpad itself: "Packagediffs need some way to be cleaned up"
  https://bugs.launchpad.net/launchpad/+bug/764263

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/remove-resolved-packagediffs-bug-764263/+merge/58283

= Summary =
Add an explicit test to make sure packagediffs are nulled out in the
distroseriesdifference row when it gets RESOLVED.

== Tests ==
No code changes, just a new test:
bin/test -cvv test_distroseriesdifference test_update_nulls_diffs_for_resolved

== Demo and Q/A ==
n/a
-- 
https://code.launchpad.net/~julian-edwards/launchpad/remove-resolved-packagediffs-bug-764263/+merge/58283
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/remove-resolved-packagediffs-bug-764263 into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py	2011-04-19 02:42:28 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py	2011-04-19 12:09:49 +0000
@@ -163,6 +163,52 @@
             DistroSeriesDifferenceStatus.RESOLVED,
             ds_diff.status)
 
+    def test_update_nulls_diffs_for_resolved(self):
+        # Resolved differences should null out the package_diff and
+        # parent_package_diff fields so the libraryfilealias gets
+        # considered for GC later.
+        derived_changelog = self.factory.makeChangelog(
+            versions=['1.0', '1.2'])
+        parent_changelog = self.factory.makeChangelog(
+            versions=['1.0', '1.3'])
+        transaction.commit() # Yay, librarian.
+        ds_diff = self.factory.makeDistroSeriesDifference(versions={
+            'derived': '1.2',
+            'parent': '1.3',
+            'base': '1.0',
+            },
+            changelogs={
+                'derived': derived_changelog,
+                'parent': parent_changelog,
+            })
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            ds_diff.requestPackageDiffs(person)
+        # The pre-test state is that there are diffs present:
+        self.assertIsNot(None, ds_diff.package_diff)
+        self.assertIsNot(None, ds_diff.parent_package_diff)
+
+        # Resolve the DSD by making the same package version published
+        # in parent and derived.
+        new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=ds_diff.source_package_name,
+            distroseries=ds_diff.derived_series,
+            status=PackagePublishingStatus.PENDING,
+            version='1.4')
+        new_parent_pub = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename=ds_diff.source_package_name,
+            distroseries=ds_diff.derived_series.parent_series,
+            status=PackagePublishingStatus.PENDING,
+            version='1.4')
+
+        # Packagediffs should be gone now.
+        was_updated = ds_diff.update()
+        self.assertTrue(was_updated)
+        self.assertEqual(
+            ds_diff.status, DistroSeriesDifferenceStatus.RESOLVED)
+        self.assertIs(None, ds_diff.package_diff)
+        self.assertIs(None, ds_diff.parent_package_diff)
+
     def test_update_re_opens_difference(self):
         # The status of a resolved difference will updated with new
         # uploads.