Merge lp:~mabac/svammel/bug-829525 into lp:svammel

[Mattias Backman]

Hi,

This branch fixes the problem caused by trying to target a series before setting a milestone. Fixed and tested by doko.

It also adds setting bug status.

Thanks,

Mattias

[James Westby (james-w)]

Hi,

Is it feasible to have a test that shows the problem with setting the milestone
after the series?

Also a test that the status is set as desired would be good.

Thanks,

James

review: Approve

[Mattias Backman (mabac)]

On Wed, Aug 24, 2011 at 5:11 PM, James Westby <email address hidden> wrote:
> Review: Approve
> Hi,
>
> Is it feasible to have a test that shows the problem with setting the milestone
> after the series?
>
> Also a test that the status is set as desired would be good.

Yes, that should be no problem. I can work on both those comment tomorrow.

>
> Thanks,
>
> James
>
> --
> https://code.launchpad.net/~mabac/svammel/bug-829525/+merge/72708
> You are the owner of lp:~mabac/svammel/bug-829525.
>

[lp:~mabac/svammel/bug-829525]

106. By Mattias Backman on 2011-09-01

Add save tests for setting status. Test that milestone is set before nominating series.

[Mattias Backman (mabac)]

Added tests for status and one for checking that milestone is set before the series nomination if both series and milestone is requested.

[James Westby (james-w)]

review: Approve

=== modified file 'bug_reporting.py'
--- bug_reporting.py	2011-04-20 14:43:31 +0000
+++ bug_reporting.py	2011-09-01 13:52:24 +0000
@@ -37,7 +37,7 @@
 
 
 def file_bug_by_url(target_url, description, title, tags, lp, importance=None,
-                    milestone_name=None, series_name=None):
+                    milestone_name=None, series_name=None, status='Confirmed'):
     """ File a bug on the bug target specified by target_url.
 
     Returns the newly created bug or None if no bug was filed.
@@ -54,7 +54,24 @@
         try:
             target = lp.distributions['ubuntu']
             logger.debug("Series and milestone target is %s." % target)
-            ms_bugtask = None
+
+            ms_bugtask = bug.bug_tasks[0]
+
+            if milestone_name is not None:
+                milestone = target.getMilestone(name=milestone_name)
+                logger.debug("Target milestone is %s." % milestone)
+                if milestone is None:
+                    raise AttributeError(
+                        "Milestone '%s' not valid for target '%s'." % \
+                            (milestone_name, target.name))
+                set_bugtask_milestone(ms_bugtask, milestone)
+
+            if status is not None:
+                set_bugtask_status(ms_bugtask, status)
+
+            if importance is not None:
+                set_bugtask_importance(ms_bugtask, importance)
+
             if series_name is not None:
                 series = target.getSeries(name_or_version=series_name)
                 logger.debug("Adding nomination for series %s." % series)
@@ -71,36 +88,12 @@
                 if nomination is not None and nomination.canApprove():
                     logger.debug("Approving nomination %s." % nomination)
                     nomination.approve()
-                    for candidate_bugtask in bug.bug_tasks:
-                        if candidate_bugtask.target == series:
-                            ms_bugtask = candidate_bugtask
-                    if ms_bugtask is None:
-                        raise Exception(
-                            "A bug task to set milestone was not found, " \
-                            "please report this bug on the svammel project.")
-
                 elif continue_on_error():
                     logger.warning("Cannot approve nomination %s.", nomination)
                 else:
                     raise AttributeError(
                         "Unable to approve nomination %s." % nomination)
 
-            if milestone_name is not None:
-                milestone = target.getMilestone(name=milestone_name)
-                logger.debug("Target milestone is %s." % milestone)
-                if milestone is None:
-                    raise AttributeError(
-                        "Milestone '%s' not valid for target '%s'." % \
-                            (milestone_name, target.name))
-
-                if ms_bugtask is None:
-                    # If series nomination is done the bug_task
-                    # has already been found.
-                    ms_bugtask = bug.bug_tasks[0]
-                set_bugtask_milestone(ms_bugtask, milestone)
-
-            if importance is not None:
-                set_bug_importance(bug, importance)
         except Exception, e:
             if continue_on_error():
                 logger.error("Error '%s' but will proceed.", e)
@@ -116,18 +109,27 @@
     return bug
 
 
-def set_bug_importance(bug, importance):
-    """Sets importance of all bug_tasks belonging to a bug."""
-    try:
-        for bug_task in bug.bug_tasks:
-            logger.debug("Setting importance %s for %s" % \
-                             (importance, bug_task.title))
-            bug_task.importance = importance
-            bug_task.lp_save()
-    except:
-        if continue_on_error():
-            logger.error("Unable to set bug importance to '%s' but " \
-                             "will proceed." % importance)
+def set_bugtask_importance(bug_task, importance):
+    """Sets importance of bug_task."""
+    try:
+        bug_task.importance = importance
+        bug_task.lp_save()
+    except:
+        if continue_on_error():
+            logger.error("Unable to set bug_task importance to '%s' but " \
+                         "will proceed." % importance)
+        else:
+            raise
+
+def set_bugtask_status(bug_task, status):
+    """Sets importance of bug_task."""
+    try:
+        bug_task.status = status
+        bug_task.lp_save()
+    except:
+        if continue_on_error():
+            logger.error("Unable to set bug_task status to '%s' but " \
+                         "will proceed." % status)
         else:
             raise
 

=== modified file 'tests/test_fail_filer.py'
--- tests/test_fail_filer.py	2011-04-20 07:21:03 +0000
+++ tests/test_fail_filer.py	2011-09-01 13:52:24 +0000
@@ -40,7 +40,8 @@
 import bug_reporting
 from bug_reporting import (
     bug_already_filed_by_url,
-    set_bug_importance,
+    set_bugtask_importance,
+    set_bugtask_status,
     set_bugtask_milestone,
     close_bug,
     file_bug_by_url,
@@ -584,6 +585,10 @@
         def add_bug_task(self, bug_task):
             self.bug_tasks.append(bug_task)
 
+        def addNomination(self, target=None):
+            assert self.bug_tasks[0].milestone is not None, (
+                "Must set milestone before nominating series.")
+
     class MockTarget:
         class MockSeries:
             def __init__(self):
@@ -599,7 +604,7 @@
         def getMilestone(self, name):
             return TestFileBug.MockMilestone(name)
 
-        def getSeries(self, name):
+        def getSeries(self, name_or_version=None):
             return self.MockSeries()
 
     class MockBugTask:
@@ -611,6 +616,7 @@
             self.target = TestFileBug.MockTarget()
             self.title = ''
             self.status = ''
+            self.milestone = None
 
         def lp_save(self):
             if self.will_fail_on_save:
@@ -633,21 +639,20 @@
 
     def test_close_bug_if_setting_importance_fails(self):
 
-        def raise_func(bug, importance):
-            self.saved_bug = bug
+        def raise_func(bugtask, importance):
+            self.saved_bugtask = bugtask
             raise self.TestFailError('Setting importance failed.')
 
         self.useFixture(MockSomethingFixture(
-                bug_reporting, 'set_bug_importance', raise_func))
+                bug_reporting, 'set_bugtask_importance', raise_func))
         try:
-            self.saved_bug = None
+            self.saved_bugtask = None
             bug = file_bug_by_url('http://dummy', 'description', 'title', [],
                                   self.MockLaunchpad([]), importance='Low')
             self.fail('Error in test case, file_bug_by_url should have '\
                           'raised an error.')
         except self.TestFailError:
-            for bug_task in self.saved_bug.bug_tasks:
-                self.assertEquals(BUG_INVALID_STATUS, bug_task.status)
+            self.assertEquals(BUG_INVALID_STATUS, self.saved_bugtask.status)
 
     def test_close_bug_if_setting_milestone_fails(self):
 
@@ -675,21 +680,29 @@
     def test_set_importance_saves(self):
         bug = self.MockBug()
         bug_task = self.MockBugTask(False)
-        bug.add_bug_task(bug_task)
-        set_bug_importance(bug, 'High')
+        set_bugtask_importance(bug_task, 'High')
         self.assertEquals(True, bug_task.has_saved)
 
     def test_set_importance_can_ignore_errors(self):
         set_ignore_errors(True)
+        bugtask = self.MockBugTask(True)
+        set_bugtask_importance(bugtask, 'High')
+
+    def test_set_status_saves(self):
         bug = self.MockBug()
-        bug.add_bug_task(self.MockBugTask(True))
-        set_bug_importance(bug, 'High')
+        bug_task = self.MockBugTask(False)
+        set_bugtask_status(bug_task, 'Confirmed')
+        self.assertEquals(True, bug_task.has_saved)
+
+    def test_set_status_can_ignore_errors(self):
+        set_ignore_errors(True)
+        bugtask = self.MockBugTask(True)
+        set_bugtask_status(bugtask, 'Confirmed')
 
     def test_set_importance_raises(self):
         set_ignore_errors(False)
-        bug = self.MockBug()
-        bug.add_bug_task(self.MockBugTask(True))
-        self.assertRaises(Exception, set_bug_importance, bug, 'High')
+        bugtask = self.MockBugTask(True)
+        self.assertRaises(Exception, set_bugtask_importance, bugtask, 'High')
 
     def test_set_milestone_saves(self):
         bug_task = self.MockBugTask(False)
@@ -716,6 +729,12 @@
         bug_task = self.MockBugTask(True)
         self.assertRaises(Exception, set_bugtask_milestone, bug_task, None)
 
+    def test_set_milestone_before_series(self):
+        failure = self.assertRaises(AttributeError, file_bug_by_url,
+                                    'http://dummy', 'description', 'title', [],
+                              self.MockLaunchpad([]), milestone_name='ms',
+                              series_name='series')
+        self.assertEqual(str(failure), 'Unable to approve nomination None.')
 
 class TestConfigInit(TestCaseWithFixtures):