Merge lp:~gmb/launchpad/comment-error-reporting-bug-571692-a into lp:launchpad

[Graham Binns]

Errors encountered when syncing comments or back-linking to remote bugs will now be handled and reported correctly rather than being returned as "bug watch couldn't be synchronised."

This branch improves the handling and importing of errors that occur
when trying to sync comments with remote bugs. At the moment, those
errors aren't reported properly (i.e. they're masked as "Bug watch
failed to update" errors, which are useless. Once this branch lands the
errors will be displayed clearly on the bug watch pages.o

No lint.

== lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py ==

 - I've updated BugWatchUpdater.updateBugWatch() so that it now records
   errors correctly if something goes wrong when syncing comments or
   back-linking.

== lib/lp/bugs/scripts/checkwatches/core.py ==

 - I've tidied up some lint.

== lib/lp/bugs/scripts/checkwatches/remotebugupdater.py ==

 - I've altered RemoteBugUpdater.updateRemoteBug() so that it handles
   errors that occur when importing remote statuses an importances
   explicitly before relying on BugWatchUpdater to deal with errors that
   occur during comment syncing.

== lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py ==

 - I've added tests to cover error handling when syncing comments and
   back-linking.
 - I've added tests to cover the fact that the can_* booleans should be
   inherited from RemoteBugUpdater.

== lib/lp/testing/factory.py ==

 - I've update makeBugWatch() to allow it to accept a bug_task argument.
 - I've updated makeBugComment to allow it to accept a bug_watch
   argument.

[Paul Hummer (rockstar)]

review: Approve (code)

=== modified file 'lib/lp/bugs/doc/externalbugtracker-comment-imports.txt'
--- lib/lp/bugs/doc/externalbugtracker-comment-imports.txt	2010-04-26 12:01:20 +0000
+++ lib/lp/bugs/doc/externalbugtracker-comment-imports.txt	2010-05-14 14:06:33 +0000
@@ -86,16 +86,13 @@
 comments in the comment_dict being imported into Launchpad.
 
     >>> from canonical.launchpad.scripts.logger import log
-    >>> from lp.bugs.scripts.checkwatches.base import (
-    ...     WorkingBase)
-    >>> from lp.bugs.scripts.checkwatches.bugwatchupdater import (
-    ...     BugWatchUpdater)
+    >>> from lp.bugs.scripts.checkwatches.core import CheckwatchesMaster
+    >>> from lp.bugs.scripts.checkwatches.tests.test_bugwatchupdater import (
+    ...     make_bug_watch_updater)
 
-    >>> working_base = WorkingBase()
-    >>> working_base.init(
-    ...     '[e-mail]', transaction, log)
-    >>> bugwatch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bugwatch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
     >>> bugwatch_updater.importBugComments()
     INFO:...:Imported 3 comments for remote bug 123456 on ...
 
@@ -396,8 +393,9 @@
     >>> LaunchpadZopelessLayer.switchDbUser(config.checkwatches.dbuser)
     >>> external_bugtracker.remote_comments = {
     ...     '5':"A comment containing a CVE entry: CVE-1991-9911."}
-    >>> bugwatch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bugwatch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
     >>> bugwatch_updater.importBugComments()
     INFO:...:Imported 1 comments for remote bug 123456...
 
@@ -470,8 +468,9 @@
 
     >>> transaction.commit()
 
-    >>> bugwatch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bugwatch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
     >>> bugwatch_updater.importBugComments()
     INFO:...:Imported 2 comments for remote bug 42 ...
 

=== modified file 'lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt'
--- lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt	2010-04-26 12:15:05 +0000
+++ lib/lp/bugs/doc/externalbugtracker-comment-pushing.txt	2010-05-14 14:06:33 +0000
@@ -96,16 +96,13 @@
 tracker.
 
     >>> from canonical.launchpad.scripts.logger import log
-    >>> from lp.bugs.scripts.checkwatches.base import (
-    ...     WorkingBase)
-    >>> from lp.bugs.scripts.checkwatches.bugwatchupdater import (
-    ...     BugWatchUpdater)
+    >>> from lp.bugs.scripts.checkwatches.core import CheckwatchesMaster
+    >>> from lp.bugs.scripts.checkwatches.tests.test_bugwatchupdater import (
+    ...     make_bug_watch_updater)
 
-    >>> working_base = WorkingBase()
-    >>> working_base.init(
-    ...     '[e-mail]', transaction, log)
-    >>> bugwatch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bugwatch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
 
     >>> bugwatch_updater.pushBugComments()
     Comment added as remote comment 1
@@ -270,8 +267,9 @@
     >>> broken_external_bugtracker = ErroringExternalBugTracker(
     ...     'http://example.com')
 
-    >>> bugwatch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bugwatch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
     >>> bugwatch_updater.external_bugtracker = broken_external_bugtracker
     >>> bugwatch_updater.pushBugComments()
     Traceback (most recent call last):

=== modified file 'lib/lp/bugs/doc/externalbugtracker-linking-back.txt'
--- lib/lp/bugs/doc/externalbugtracker-linking-back.txt	2010-04-26 12:15:05 +0000
+++ lib/lp/bugs/doc/externalbugtracker-linking-back.txt	2010-05-14 14:06:33 +0000
@@ -91,17 +91,13 @@
 
     >>> transaction.commit()
 
-    >>> from canonical.launchpad.scripts.logger import log
-    >>> from lp.bugs.scripts.checkwatches.base import (
-    ...     WorkingBase)
-    >>> from lp.bugs.scripts.checkwatches.bugwatchupdater import (
-    ...     BugWatchUpdater)
+    >>> from lp.bugs.scripts.checkwatches.core import CheckwatchesMaster
+    >>> from lp.bugs.scripts.checkwatches.tests.test_bugwatchupdater import (
+    ...     make_bug_watch_updater)
 
-    >>> working_base = WorkingBase()
-    >>> working_base.init(
-    ...     '[e-mail]', transaction, log)
-    >>> bug_watch_updater = BugWatchUpdater(
-    ...     working_base, bug_watch, external_bugtracker)
+    >>> bug_watch_updater = make_bug_watch_updater(
+    ...     CheckwatchesMaster(transaction), bug_watch,
+    ...     external_bugtracker)
     >>> bug_watch_updater.linkLaunchpadBug()
     Getting Launchpad id for bug 42
 

=== modified file 'lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py'
--- lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py	2010-04-27 10:32:21 +0000
+++ lib/lp/bugs/scripts/checkwatches/bugwatchupdater.py	2010-05-14 14:06:33 +0000
@@ -24,6 +24,7 @@
 from lazr.lifecycle.event import ObjectCreatedEvent
 
 from lp.bugs.interfaces.bug import IBugSet
+from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
 from lp.bugs.scripts.checkwatches.base import (
     WorkingBase, commit_before)
 from lp.bugs.scripts.checkwatches.utilities import (
@@ -31,7 +32,6 @@
 from lp.registry.interfaces.person import PersonCreationRationale
 
 
-
 class BugWatchUpdater(WorkingBase):
     """Handles the updating of a single BugWatch for checkwatches."""
 
@@ -40,26 +40,41 @@
         self.bug_watch = bug_watch
         self.external_bugtracker = external_bugtracker
 
+        # We save these for the sake of error reporting.
+        self.remote_bug = self.bug_watch.remotebug
+        self.local_bug = self.bug_watch.bug.id
+        self.oops_properties = get_remote_system_oops_properties(
+            self.external_bugtracker)
+        self.oops_properties.extend([
+            ('URL', self.bug_watch.url),
+            ('bug_id', self.remote_bug),
+            ('local_ids', str(self.local_bug))])
+
+        self.can_import_comments = parent.can_import_comments
+        self.can_push_comments = parent.can_push_comments
+        self.can_back_link = parent.can_back_link
+
+    # XXX 2010-05-11 gmb bug=578714:
+    #     The last three parameters on this method aren't needed and
+    #     should be removed.
     @commit_before
     def updateBugWatch(self, new_remote_status, new_malone_status,
                        new_remote_importance, new_malone_importance,
-                       can_import_comments, can_push_comments, can_back_link,
-                       error, oops_id):
+                       can_import_comments=None, can_push_comments=None,
+                       can_back_link=None):
         """Update the BugWatch."""
         with self.transaction:
-            self.bug_watch.last_error_type = error
             if new_malone_status is not None:
                 self.bug_watch.updateStatus(
                     new_remote_status, new_malone_status)
             if new_malone_importance is not None:
                 self.bug_watch.updateImportance(
                     new_remote_importance, new_malone_importance)
-            # Only sync comments and backlink if there was no
-            # earlier error, the local bug isn't a duplicate,
-            # *and* if the bug watch is associated with a bug
-            # task. This helps us to avoid spamming upstream.
+            # Only sync comments and backlink if the local bug isn't a
+            # duplicate and the bug watch is associated with a bug task.
+            # This helps us to avoid spamming both upstream and
+            # ourselves.
             do_sync = (
-                error is None and
                 self.bug_watch.bug.duplicateof is None and
                 len(self.bug_watch.bugtasks) > 0
                 )
@@ -69,17 +84,35 @@
         # throw an exception, *all* the watches in
         # self.bug_watches, even those that have not errored,
         # will have negative activity added.
+        error_message = None
+        error_status = None
+        oops_id = None
         if do_sync:
-            if can_import_comments:
-                self.importBugComments()
-            if can_push_comments:
-                self.pushBugComments()
-            if can_back_link:
-                self.linkLaunchpadBug()
+            try:
+                if can_import_comments or self.can_import_comments:
+                    error_status = (
+                        BugWatchActivityStatus.COMMENT_IMPORT_FAILED)
+                    self.importBugComments()
+                if can_push_comments or self.can_push_comments:
+                    error_status = BugWatchActivityStatus.COMMENT_PUSH_FAILED
+                    self.pushBugComments()
+                if can_back_link or self.can_back_link:
+                    error_status = BugWatchActivityStatus.BACKLINK_FAILED
+                    self.linkLaunchpadBug()
+            except Exception, ex:
+                error_message = ex.message
+                oops_id = self.error(
+                    "Failure updating bug %r on %s (local bug: %s)." %
+                        (self.remote_bug, self.external_bugtracker.baseurl,
+                        self.local_bug),
+                    self.oops_properties)
+            else:
+                error_status = None
 
         with self.transaction:
             self.bug_watch.addActivity(
-                result=error, oops_id=oops_id)
+                result=error_status, message=error_message, oops_id=oops_id)
+            self.bug_watch.last_error_type = error_status
 
     @commit_before
     def importBugComments(self):

=== modified file 'lib/lp/bugs/scripts/checkwatches/core.py'
--- lib/lp/bugs/scripts/checkwatches/core.py	2010-05-06 10:32:58 +0000
+++ lib/lp/bugs/scripts/checkwatches/core.py	2010-05-14 14:06:33 +0000
@@ -34,19 +34,16 @@
 
 from zope.component import getUtility
 
-from canonical.database.constants import UTC_NOW
 from canonical.database.sqlbase import flush_database_updates
 from canonical.launchpad.interfaces import (
     CreateBugParams, IBugTrackerSet, IBugWatchSet, IDistribution,
-    ILaunchpadCelebrities, IPersonSet, ISupportsCommentImport,
-    ISupportsCommentPushing, PersonCreationRationale)
+    ILaunchpadCelebrities, IPersonSet, PersonCreationRationale)
 from canonical.launchpad.scripts.logger import log as default_log
 
 from lp.bugs import externalbugtracker
 from lp.bugs.externalbugtracker import (
     BATCH_SIZE_UNLIMITED, BugWatchUpdateError,
     UnknownBugTrackerTypeError)
-from lp.bugs.interfaces.externalbugtracker import ISupportsBackLinking
 from lp.bugs.scripts.checkwatches.base import (
     WorkingBase, commit_before, with_interaction)
 from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater

=== modified file 'lib/lp/bugs/scripts/checkwatches/remotebugupdater.py'
--- lib/lp/bugs/scripts/checkwatches/remotebugupdater.py	2010-05-04 10:27:44 +0000
+++ lib/lp/bugs/scripts/checkwatches/remotebugupdater.py	2010-05-14 14:06:33 +0000
@@ -155,15 +155,23 @@
                             self.external_bugtracker),
                     info=sys.exc_info())
 
-            for bug_watch in bug_watches:
-                bug_watch_updater = BugWatchUpdater(
-                    self, bug_watch, self.external_bugtracker)
-
-                bug_watch_updater.updateBugWatch(
-                    new_remote_status, new_malone_status,
-                    new_remote_importance, new_malone_importance,
-                    self.can_import_comments, self.can_push_comments,
-                    self.can_back_link, error, oops_id)
+                # Set the error and activity on all bug watches
+                with self.transaction:
+                    getUtility(IBugWatchSet).bulkSetError(
+                        bug_watches, error)
+                    getUtility(IBugWatchSet).bulkAddActivity(
+                        bug_watches, result=error, oops_id=oops_id)
+
+            else:
+                # Assuming nothing's gone wrong, we can now deal with
+                # each BugWatch in turn.
+                for bug_watch in bug_watches:
+                    bug_watch_updater = BugWatchUpdater(
+                        self, bug_watch, self.external_bugtracker)
+
+                    bug_watch_updater.updateBugWatch(
+                        new_remote_status, new_malone_status,
+                        new_remote_importance, new_malone_importance)
 
         except Exception, error:
             # Send the error to the log.

=== added file 'lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py'
--- lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/scripts/checkwatches/tests/test_bugwatchupdater.py	2010-05-14 14:06:33 +0000
@@ -0,0 +1,214 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the checkwatches.bugwatchupdater module."""
+
+__metaclass__ = type
+
+import transaction
+import unittest
+
+from datetime import datetime
+
+from canonical.testing import LaunchpadZopelessLayer
+
+from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI
+from lp.bugs.interfaces.bugtask import BugTaskImportance, BugTaskStatus
+from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
+from lp.bugs.scripts.checkwatches.bugwatchupdater import BugWatchUpdater
+from lp.bugs.scripts.checkwatches.remotebugupdater import RemoteBugUpdater
+from lp.bugs.scripts.checkwatches.core import CheckwatchesMaster
+from lp.bugs.tests.externalbugtracker import TestExternalBugTracker
+from lp.testing import TestCaseWithFactory
+
+
+def make_bug_watch_updater(checkwatches_master, bug_watch,
+                           external_bugtracker, server_time=None):
+    """Helper function to create a BugWatchUpdater instance."""
+    if server_time is None:
+        server_time = datetime.now()
+
+    remote_bug_updater = checkwatches_master.remote_bug_updater_factory(
+        checkwatches_master, external_bugtracker, bug_watch.remotebug,
+        [bug_watch.id], [], server_time)
+    return BugWatchUpdater(
+        remote_bug_updater, bug_watch,
+        remote_bug_updater.external_bugtracker)
+
+
+class BrokenCommentSyncingExternalBugTracker(TestExternalBugTracker):
+    """An ExternalBugTracker that can't sync comments."""
+
+    import_comments_error_message = "Can't import comments, sorry."
+    push_comments_error_message = "Can't push comments, sorry."
+    back_link_error_message = "Can't back link, sorry."
+
+    def getCommentIds(self, remote_bug_id):
+        raise Exception(self.import_comments_error_message)
+
+    def addRemoteComment(self, remote_bug_id, formatted_comment, message_id):
+        raise Exception(self.push_comments_error_message)
+
+    def getLaunchpadBugId(self, remote_bug):
+        raise Exception(self.back_link_error_message)
+
+
+class LoggingBugWatchUpdater(BugWatchUpdater):
+    """A BugWatchUpdater that logs what's going on."""
+
+    import_bug_comments_called = False
+    push_bug_comments_called = False
+    link_launchpad_bug_called = False
+
+    def importBugComments(self):
+        self.import_bug_comments_called = True
+
+    def pushBugComments(self):
+        self.push_bug_comments_called = True
+
+    def linkLaunchpadBug(self):
+        self.link_launchpad_bug_called = True
+
+
+class BugWatchUpdaterTestCase(TestCaseWithFactory):
+    """Tests the functionality of the BugWatchUpdater class."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(BugWatchUpdaterTestCase, self).setUp()
+        self.checkwatches_master = CheckwatchesMaster(transaction)
+        self.bug_task = self.factory.makeBugTask()
+        self.bug_watch = self.factory.makeBugWatch(bug_task=self.bug_task)
+
+    def _checkLastErrorAndMessage(self, expected_last_error,
+                                  expected_message):
+        """Check the latest activity and last_error_type for a BugWatch."""
+        latest_activity = self.bug_watch.activity[0]
+        self.assertEqual(expected_last_error, self.bug_watch.last_error_type)
+        self.assertEqual(expected_last_error, latest_activity.result)
+        self.assertEqual(expected_message, latest_activity.message)
+
+    def test_updateBugWatch(self):
+        # Calling BugWatchUpdater.updateBugWatch() will update the
+        # updater's current BugWatch.
+        bug_watch_updater = make_bug_watch_updater(
+            self.checkwatches_master, self.bug_watch,
+            TestExternalBugTracker('http://example.com'))
+
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
+            BugTaskImportance.LOW, can_import_comments=False,
+            can_push_comments=False, can_back_link=False)
+
+        self.assertEqual('FIXED', self.bug_watch.remotestatus)
+        self.assertEqual(BugTaskStatus.FIXRELEASED, self.bug_task.status)
+        self.assertEqual('LOW', self.bug_watch.remote_importance)
+        self.assertEqual(BugTaskImportance.LOW, self.bug_task.importance)
+        self.assertEqual(None, self.bug_watch.last_error_type)
+
+        latest_activity = self.bug_watch.activity[0]
+        self.assertEqual(
+            BugWatchActivityStatus.SYNC_SUCCEEDED, latest_activity.result)
+
+    def test_importBugComments_error_handling(self):
+        # If an error occurs when importing bug comments, it will be
+        # recorded as BugWatchActivityStatus.COMMENT_IMPORT_FAILED.
+        external_bugtracker = BrokenCommentSyncingExternalBugTracker(
+            'http://example.com')
+        bug_watch_updater = make_bug_watch_updater(
+            self.checkwatches_master, self.bug_watch, external_bugtracker)
+
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
+            BugTaskImportance.LOW, can_import_comments=True,
+            can_push_comments=False, can_back_link=False)
+
+        self._checkLastErrorAndMessage(
+            BugWatchActivityStatus.COMMENT_IMPORT_FAILED,
+            external_bugtracker.import_comments_error_message)
+
+    def test_pushBugComments_error_handling(self):
+        # If an error occurs when pushing bug comments, it will be
+        # recorded as BugWatchActivityStatus.COMMENT_IMPORT_FAILED.
+        external_bugtracker = BrokenCommentSyncingExternalBugTracker(
+            'http://example.com')
+        bug_watch_updater = make_bug_watch_updater(
+            self.checkwatches_master, self.bug_watch, external_bugtracker)
+
+        self.factory.makeBugComment(
+            bug=self.bug_task.bug, bug_watch=self.bug_watch)
+
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
+            BugTaskImportance.LOW, can_import_comments=False,
+            can_push_comments=True, can_back_link=False)
+
+        self._checkLastErrorAndMessage(
+            BugWatchActivityStatus.COMMENT_PUSH_FAILED,
+            external_bugtracker.push_comments_error_message)
+
+    def test_linkLaunchpadBug_error_handling(self):
+        # If an error occurs when linking back to a remote bug, it will
+        # be recorded as BugWatchActivityStatus.BACKLINK_FAILED.
+        external_bugtracker = BrokenCommentSyncingExternalBugTracker(
+            'http://example.com')
+        bug_watch_updater = make_bug_watch_updater(
+            self.checkwatches_master, self.bug_watch, external_bugtracker)
+
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW',
+            BugTaskImportance.LOW, can_import_comments=False,
+            can_push_comments=False, can_back_link=True)
+
+        self._checkLastErrorAndMessage(
+            BugWatchActivityStatus.BACKLINK_FAILED,
+            external_bugtracker.back_link_error_message)
+
+    def test_comment_bools_inherited(self):
+        # BugWatchUpdater.updateBugWatches() doesn't have to be passed
+        # values for can_import_comments, can_push_comments and
+        # can_back_link. Instead, these can be taken from the parent
+        # object passed to it upon instantiation, usually a
+        # RemoteBugUpdater.
+        # XXX 2010-05-11 gmb bug=578714:
+        #     This test can be removed when bug 578714 is fixed.
+        remote_bug_updater = RemoteBugUpdater(
+            self.checkwatches_master,
+            BugzillaAPI("http://example.com"),
+            self.bug_watch.remotebug, [self.bug_watch.id],
+            [], datetime.now())
+        bug_watch_updater = LoggingBugWatchUpdater(
+            remote_bug_updater, self.bug_watch,
+            remote_bug_updater.external_bugtracker)
+
+        # If all the can_* properties of remote_bug_updater are True,
+        # bug_watch_updater will attempt to import, push and backlink.
+        self.assertTrue(remote_bug_updater.can_import_comments)
+        self.assertTrue(remote_bug_updater.can_push_comments)
+        self.assertTrue(remote_bug_updater.can_back_link)
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW', BugTaskImportance.LOW)
+
+        self.assertTrue(bug_watch_updater.import_bug_comments_called)
+        self.assertTrue(bug_watch_updater.push_bug_comments_called)
+        self.assertTrue(bug_watch_updater.link_launchpad_bug_called)
+
+        # Otherwise, bug_watch_updater won't attempt those functions
+        # whose can_* properties are False.
+        remote_bug_updater.can_import_comments = False
+        remote_bug_updater.can_push_comments = False
+        remote_bug_updater.can_back_link = False
+        bug_watch_updater = LoggingBugWatchUpdater(
+            remote_bug_updater, self.bug_watch,
+            remote_bug_updater.external_bugtracker)
+        bug_watch_updater.updateBugWatch(
+            'FIXED', BugTaskStatus.FIXRELEASED, 'LOW', BugTaskImportance.LOW)
+
+        self.assertFalse(bug_watch_updater.import_bug_comments_called)
+        self.assertFalse(bug_watch_updater.push_bug_comments_called)
+        self.assertFalse(bug_watch_updater.link_launchpad_bug_called)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-05-08 12:30:49 +0000
+++ lib/lp/testing/factory.py	2010-05-14 14:06:33 +0000
@@ -1169,7 +1169,7 @@
             base_url, owner, bugtrackertype)
 
     def makeBugWatch(self, remote_bug=None, bugtracker=None, bug=None,
-                     owner=None):
+                     owner=None, bug_task=None):
         """Make a new bug watch."""
         if remote_bug is None:
             remote_bug = self.getUniqueInteger()
@@ -1177,7 +1177,14 @@
         if bugtracker is None:
             bugtracker = self.makeBugTracker()
 
-        if bug is None:
+        if bug_task is not None:
+            # If someone passes a value for bug *and* a value for
+            # bug_task then the bug value will get clobbered, but that
+            # doesn't matter since the bug should be the one that the
+            # bug task belongs to anyway (unless they're having a crazy
+            # moment, in which case we're saving them from themselves).
+            bug = bug_task.bug
+        elif bug is None:
             bug = self.makeBug()
 
         if owner is None:
@@ -1185,6 +1192,8 @@
 
         bug_watch = getUtility(IBugWatchSet).createBugWatch(
             bug, owner, bugtracker, str(remote_bug))
+        if bug_task is not None:
+            bug_task.bugwatch = bug_watch
 
         # You need to be an admin to set next_check on a BugWatch.
         def set_next_check(bug_watch):
@@ -1194,7 +1203,8 @@
         run_with_login(person, set_next_check, bug_watch)
         return bug_watch
 
-    def makeBugComment(self, bug=None, owner=None, subject=None, body=None):
+    def makeBugComment(self, bug=None, owner=None, subject=None, body=None,
+                       bug_watch=None):
         """Create and return a new bug comment.
 
         :param bug: An `IBug` or a bug ID or name, or None, in which
@@ -1205,6 +1215,8 @@
             case a new message will be generated.
         :param body: An `IMessage` or a string, or None, in which
             case a new message will be generated.
+        :param bug_watch: An `IBugWatch`, which will be used to set the
+            new comment's bugwatch attribute.
         :return: An `IBugMessage`.
         """
         if bug is None:
@@ -1218,7 +1230,7 @@
         if body is None:
             body = self.getUniqueString()
         return bug.newMessage(owner=owner, subject=subject,
-                              content=body, parent=None, bugwatch=None,
+                              content=body, parent=None, bugwatch=bug_watch,
                               remote_comment_id=None)
 
     def makeBugAttachment(self, bug=None, owner=None, data=None,