launchpad-reviewers team mailing list archive

[Bryce Harrington <bryce@xxxxxxxxxxxxx>]

Bryce Harrington has proposed merging lp:~bryce/launchpad/bugtracker_component_test_refactor into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~bryce/launchpad/bugtracker_component_test_refactor/+merge/65083

Test code refactoring for components.  These mostly comprise Jeroen's suggestions from another branch review, split out for merge simplification.
-- 
https://code.launchpad.net/~bryce/launchpad/bugtracker_component_test_refactor/+merge/65083
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bryce/launchpad/bugtracker_component_test_refactor into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/tests/test_bugtracker_component.py'
--- lib/lp/bugs/browser/tests/test_bugtracker_component.py	2011-06-03 05:47:32 +0000
+++ lib/lp/bugs/browser/tests/test_bugtracker_component.py	2011-06-18 04:47:25 +0000
@@ -40,13 +40,18 @@
             'field.actions.save': 'Save',
             }
 
+    def _makeComponent(self, name):
+        return self.factory.makeBugTrackerComponent(name, self.comp_group)
+
+    def _makeUbuntuSourcePackage(self, package_name):
+        distro = getUtility(IDistributionSet).getByName('ubuntu')
+        return self.factory.makeDistributionSourcePackage(
+            sourcepackagename=package_name, distribution=distro)
+
     def test_view_attributes(self):
-        component = self.factory.makeBugTrackerComponent(
-            u'Example', self.comp_group)
-        distro = getUtility(IDistributionSet).getByName('ubuntu')
-        package = self.factory.makeDistributionSourcePackage(
-            sourcepackagename='example', distribution=distro)
-        form = self._makeForm(package)
+        component = self._makeComponent(u'Example')
+        dsp = self._makeUbuntuSourcePackage('example')
+        form = self._makeForm(dsp)
         view = create_initialized_view(
             component, name='+edit', form=form)
         label = 'Link a distribution source package to Example component'
@@ -58,33 +63,38 @@
         self.assertEqual(url, view.cancel_url)
 
     def test_linking(self):
-        component = self.factory.makeBugTrackerComponent(
-            u'Example', self.comp_group)
-        distro = getUtility(IDistributionSet).getByName('ubuntu')
-        package = self.factory.makeDistributionSourcePackage(
-            sourcepackagename='example', distribution=distro)
+        component = self._makeComponent(u'Example')
+        dsp = self._makeUbuntuSourcePackage('example')
+        form = self._makeForm(dsp)
 
         self.assertIs(None, component.distro_source_package)
-        form = self._makeForm(package)
+        view = create_initialized_view(
+            component, name='+edit', form=form)
+        notifications = view.request.response.notifications
+        self.assertEqual(dsp, component.distro_source_package)
+
+    def test_linking_notifications(self):
+        component = self._makeComponent(u'Example')
+        dsp = self._makeUbuntuSourcePackage('example')
+        form = self._makeForm(dsp)
+
         view = create_initialized_view(
             component, name='+edit', form=form)
         self.assertEqual([], view.errors)
-
         notifications = view.request.response.notifications
-        self.assertEqual(component.distro_source_package, package)
-        expected = (
-            u"alpha:Example is now linked to the example "
-            "source package in ubuntu.")
-        self.assertEqual(expected, notifications.pop().message)
+        expected = """
+            alpha:Example is now linked to the example
+            source package in ubuntu."""
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            expected, notifications.pop().message)
 
     def test_unlinking(self):
-        component = self.factory.makeBugTrackerComponent(
-            u'Example', self.comp_group)
+        component = self._makeComponent(u'Example')
         distro = getUtility(IDistributionSet).getByName('ubuntu')
-        dsp = self.factory.makeDistributionSourcePackage(
-            sourcepackagename='example', distribution=distro)
+        dsp = self._makeUbuntuSourcePackage('example')
         component.distro_source_package = dsp
         form = self._makeForm(None)
+
         view = create_initialized_view(
             component, name='+edit', form=form)
         self.assertEqual([], view.errors)
@@ -94,31 +104,28 @@
         self.assertEqual(expected, notifications.pop().message)
 
     def test_cannot_doublelink_sourcepackages(self):
-        # Two components try linking to same package
-        component_a = self.factory.makeBugTrackerComponent(
-            u'a', self.comp_group)
-        component_b = self.factory.makeBugTrackerComponent(
-            u'b', self.comp_group)
-        distro = getUtility(IDistributionSet).getByName('ubuntu')
-        package = self.factory.makeDistributionSourcePackage(
-            sourcepackagename='example', distribution=distro)
-
-        form = self._makeForm(package)
-        view = create_initialized_view(
-            component_a, name='+edit', form=form)
-        notifications = view.request.response.notifications
-        self.assertEqual([], view.errors)
-        self.assertEqual(1, len(notifications))
-        self.assertEqual(package, component_a.distro_source_package)
-
-        form = self._makeForm(package)
+        ''' Two components try linking to same same package
+
+        We must maintain a one-to-one relationship between components
+        and source packages.  However, users are bound to attempt to try
+        to make multiple components linked to the same source package,
+        so the view needs to be sure to not allow this to be done and
+        pop up a friendly error message instead.
+        '''
+        component_a = self._makeComponent(u'a')
+        component_b = self._makeComponent(u'b')
+        package = self._makeUbuntuSourcePackage('example')
+        form = self._makeForm(package)
+
+        component_a.distro_source_package = package
         view = create_initialized_view(
             component_b, name='+edit', form=form)
         self.assertIs(None, component_b.distro_source_package)
         self.assertEqual([], view.errors)
         notifications = view.request.response.notifications
         self.assertEqual(1, len(notifications))
-        expected = (
-            "The example source package is already linked to "
-            "alpha:a in ubuntu.")
-        self.assertEqual(expected, notifications.pop().message)
+        expected = """
+            The example source package is already linked to
+            alpha:a in ubuntu."""
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            expected, notifications.pop().message)

=== modified file 'lib/lp/bugs/doc/bugtracker.txt'
--- lib/lp/bugs/doc/bugtracker.txt	2011-05-27 19:53:20 +0000
+++ lib/lp/bugs/doc/bugtracker.txt	2011-06-18 04:47:25 +0000
@@ -324,14 +324,9 @@
 Filing a bug on the remote tracker
 ----------------------------------
 
-The IBugTracker interface defines a method, getBugFilingAndSearchLinks(),
-which returns the URLs of the bug filing form and the bug search on
-the remote bug tracker as a dict. It accepts three parameters:
-remote_product, which is the name of the product on the remote
-tracker; summary, which is the bug summary to be passed to the remote
-bug tracker for searching or filing and description, which is the full
-description of the bug. This is only passed to the bug filing form as
-it is too specific for the search form.
+The IBugTracker interface defines a method to convert product,
+component, summary, and description strings into URLs for filing and/or
+searching bugs.
 
     >>> def print_links(links_dict):
     ...     for key in sorted(links_dict):