Merge lp:~adam-collard/tarmac/milestone-filter-by-series into lp:tarmac

[Adam Collard]

When setting the milestone, only look for milestones which match the series that the bug has been committed on.

[Данило Шеган (danilo)]

LGTM (tests, lint all pass as well). FWIW, the default collections batch size LP seems to use is 75 items, so this is needed when you've got more milestones than that.

review: Approve

[Chris Glass (tribaal)]

+1 !

review: Approve

[lp:~adam-collard/tarmac/milestone-filter-by-series]

435. By Adam Collard on 2015-12-02

No need to walk next_collection links

[dobey (dobey)]

Running the tests with ./run-tests in the tree on my 14.04 system, There are 2 failures and 9 errors:

======================================================================
ERROR: test__find_milestone_no_dates (tarmac.plugins.tests.test_bugresolver.BugResolverTests)
tarmac.plugins.tests.test_bugresolver.BugResolverTests.test__find_milestone_no_dates
----------------------------------------------------------------------
_StringException: Empty attachments:
  log

Traceback (most recent call last):
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/tests/test_bugresolver.py", line 233, in test__find_milestone_no_dates
    milestones = list(self.plugin._find_milestones(self.series[1]))
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/bugresolver.py", line 116, in _find_milestones
    if milestone.name == default:
AttributeError: 'int' object has no attribute 'name'

======================================================================
ERROR: test__find_milestone_specific_negative (tarmac.plugins.tests.test_bugresolver.BugResolverTests)
tarmac.plugins.tests.test_bugresolver.BugResolverTests.test__find_milestone_specific_negative
----------------------------------------------------------------------
_StringException: Empty attachments:
  log

Traceback (most recent call last):
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/tests/test_bugresolver.py", line 226, in test__find_milestone_specific_negative
    milestones = list(self.plugin._find_milestones(self.series[0]))
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/bugresolver.py", line 116, in _find_milestones
    if milestone.name == default:
AttributeError: 'int' object has no attribute 'name'

======================================================================
ERROR: test__find_target_milestone_between (tarmac.plugins.tests.test_bugresolver.BugResolverTests)
tarmac.plugins.tests.test_bugresolver.BugResolverTests.test__find_target_milestone_between
----------------------------------------------------------------------
_StringException: Empty attachments:
  log

Traceback (most recent call last):
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/tests/test_bugresolver.py", line 184, in test__find_target_milestone_between
    self.milestone_past.date_targeted + timedelta(weeks=1))
  File "/home/dobey/Projects/canonical/tarmac/milestone-filter-by-series/tarmac/plugins/bugresolver.py", line 142, in _find_target_milestone
    if milestone.date_targeted is None:
AttributeError: 'int' object has no attribute 'date_targeted'

======================================================================
ERROR: test__find_target_milestone_lexical_sort_past_dates (tarmac.plugins.tests.test_bugresolver.BugResolverTests)
tarmac.plugins.tests.test_bugresolver.BugResolverTests.test__find_target_milestone_lexical_sort_past_dates
----------------------------------------------------------------------
_StringException: Empty attachments:
  log

Traceback (most recent call last):
  File "/home/dobey/Projects/canonical/tar...

review: Needs Fixing

Unmerged revisions

435. By Adam Collard on 2015-12-02

No need to walk next_collection links

434. By Adam Collard on 2015-11-30

fix lint

433. By Adam Collard on 2015-11-30

Filter by series

=== modified file 'tarmac/plugins/bugresolver.py'
--- tarmac/plugins/bugresolver.py	2014-03-24 21:38:36 +0000
+++ tarmac/plugins/bugresolver.py	2015-12-02 15:37:05 +0000
@@ -57,7 +57,7 @@
 
             if task:
                 task.status = u'Fix Committed'
-                self._set_milestone_on_task(project, task)
+                self._set_milestone_on_task(series, task)
                 task.lp_save()
             else:
                 self.logger.info('Target %s/%s not found in bug #%s.',
@@ -82,7 +82,7 @@
             "set_milestone": set_milestone,
             "default_milestone": default_milestone}
 
-    def _set_milestone_on_task(self, project, task):
+    def _set_milestone_on_task(self, series, task):
         """
         Attempt to auto-determine the milestone to set, and set the milestone
         of the given task.  If the task already has a milestone set => noop.
@@ -97,27 +97,28 @@
                     task.bug.id, task.bug_target_name, task_milestone.name))
             return
         now = datetime.utcnow()
-        target_milestone = self._find_target_milestone(project, now)
+        target_milestone = self._find_target_milestone(series, now)
         self.logger.info("%s/%s: Setting Milestone: %s",
-                task.bug.id, task.bug_target_name, target_milestone)
+                         task.bug.id, task.bug_target_name, target_milestone)
         task.milestone = target_milestone
 
-    def _find_milestones(self, project):
+    def _find_milestones(self, series):
         """
-        Return list of active milestones in a project.  If the config
-        `default_milestone` is set filter by that.  If not found, an 
-        empty list will be returned.
+        Return iterable of active milestones in a series.  If
+        the config `default_milestone` is set filter by that.  If not
+        found, an empty list will be returned.
         """
         default = self.config["default_milestone"]
+        milestones = series.active_milestones
         if default is None:
-            return list(project.active_milestones)
-        for milestone in project.active_milestones:
+            return milestones
+        for milestone in milestones:
             if milestone.name == default:
                 return [milestone]
         self.logger.warning("Default Milestone not found: %s" % default)
         return []
 
-    def _find_target_milestone(self, project, now):
+    def _find_target_milestone(self, series, now):
         """
         Find a target milestone when resolving a bug task.
 
@@ -135,7 +136,7 @@
             2) least lexically sorting milestone (by name)
             3) the last milestone in the list (covers len()==1 case).
         """
-        milestones = self._find_milestones(project)
+        milestones = self._find_milestones(series)
         earliest_after = latest_before = untargeted = None
         for milestone in milestones:
             if milestone.date_targeted is None:
 
=== modified file 'tarmac/plugins/tests/test_bugresolver.py'
--- tarmac/plugins/tests/test_bugresolver.py	2014-03-24 21:38:36 +0000
+++ tarmac/plugins/tests/test_bugresolver.py	2015-12-02 15:37:05 +0000
@@ -76,15 +76,15 @@
                                  bug=Thing(id="1"),
                                  bug_target_name=self.targets[2].name)])}
         # Insert out of order to make sure they sort correctly.
-        self.milestones = [
+        self.milestones = Thing(total_size=4, entries=[
                 self.milestone_far_future, self.milestone_with_bug,
-                self.milestone_past, self.milestone_future]
-        self.milestones_extended = [
+                self.milestone_past, self.milestone_future])
+        self.milestones_extended = Thing(total_size=3, entries=[
                 self.milestone_untargeted_c, self.milestone_untargeted_b,
-                self.milestone_untargeted_a]
-        self.milestones_extended.extend(self.milestones)
-        self.projects[0].active_milestones = self.milestones
-        self.projects[1].active_milestones = self.milestones_extended
+                self.milestone_untargeted_a])
+        self.milestones_extended.entries.extend(self.milestones.entries)
+        self.series[0].active_milestones = self.milestones
+        self.series[1].active_milestones = self.milestones_extended
 
     def getSeries(self, name=None):
         """Faux getSeries for testing."""
@@ -124,7 +124,7 @@
         launchpad = Thing(bugs=self.bugs)
         command = Thing(launchpad=launchpad)
         self.plugin.run(command=command, target=target, source=None,
-                            proposal=self.proposal)
+                        proposal=self.proposal)
         self.assertEqual(self.bugs['0'].bug_tasks[0].milestone,
                          self.milestone_future)
         self.assertEqual(self.bugs['1'].bug_tasks[0].milestone,
@@ -173,14 +173,14 @@
     def test__find_target_milestone_older(self):
         """Dates before all milestones return the oldest milestone."""
         milestone = self.plugin._find_target_milestone(
-            self.projects[0],
+            self.series[0],
             self.milestone_past.date_targeted - timedelta(weeks=1))
         self.assertEqual(milestone, self.milestone_past)
 
     def test__find_target_milestone_between(self):
         """Test that dates between milestones return the closest newest."""
         milestone = self.plugin._find_target_milestone(
-            self.projects[1],
+            self.series[1],
             self.milestone_past.date_targeted + timedelta(weeks=1))
         self.assertTrue(self.now < self.milestone_future.date_targeted)
         self.assertTrue(self.now > self.milestone_past.date_targeted)
@@ -189,14 +189,14 @@
     def test__find_target_milestone_newer(self):
         """Test that dates after all milestones return the newest."""
         milestone = self.plugin._find_target_milestone(
-            self.projects[0],
+            self.series[0],
             self.milestone_far_future.date_targeted + timedelta(weeks=1))
         self.assertEqual(milestone, self.milestone_far_future)
 
     def test__find_target_milestone_lexical_sort_past_dates(self):
         """Dates after milestones return the least sorted no-expected-date."""
         milestone = self.plugin._find_target_milestone(
-            self.projects[1],
+            self.series[1],
             self.milestone_far_future.date_targeted + timedelta(weeks=1))
         self.assertEqual(milestone, self.milestone_untargeted_a)
 
@@ -204,32 +204,33 @@
         """Test that specifying a default gets a specific milestone."""
         self.plugin.config["default_milestone"] = "c"
         milestone = self.plugin._find_target_milestone(
-            self.projects[1],
+            self.series[1],
             self.milestone_far_future.date_targeted + timedelta(weeks=1))
         self.assertEqual(milestone, self.milestone_untargeted_c)
 
     def test__find_milestone_positive(self):
         """Given a project, the list of milestones is returned."""
-        milestones = self.plugin._find_milestones(self.projects[1])
+        milestones = list(self.plugin._find_milestones(self.series[1]))
         self.assertEqual(len(milestones), 7)
 
     def test__find_milestone_negative(self):
         """Given a project with no milestones, _find_milestone handles it"""
-        milestones = self.plugin._find_milestones(Thing(active_milestones=[]))
+        milestones = list(self.plugin._find_milestones(
+            Thing(active_milestones=Thing(total_size=0, entries=[]))))
         self.assertEqual(len(milestones), 0)
 
     def test__find_milestone_specific_negative(self):
         """Find a secific milestone that isn't there, check for log"""
         self.plugin.logger.warning = MagicMock()
         self.plugin.config["default_milestone"] = "FOO"
-        milestones = self.plugin._find_milestones(self.projects[0])
+        milestones = list(self.plugin._find_milestones(self.series[0]))
         self.assertEqual(len(milestones), 0)
         self.assertEqual(self.plugin.logger.warning.call_count, 1)
 
     def test__find_milestone_no_dates(self):
         """Find a specific milestones without a targeted date"""
         self.plugin.config["default_milestone"] = "b"
-        milestones = self.plugin._find_milestones(self.projects[1])
+        milestones = list(self.plugin._find_milestones(self.series[1]))
         self.assertEqual(len(milestones), 1)
         self.assertEqual(milestones[0], self.milestone_untargeted_b)
 
@@ -277,7 +278,7 @@
         self.plugin.config = {
             "set_milestone": False, "default_milestone": None}
         self.plugin._set_milestone_on_task(
-            self.projects[0], self.bugs['0'].bug_tasks[0])
+            self.series[0], self.bugs['0'].bug_tasks[0])
         self.assertEqual(self.bugs['0'].bug_tasks[0].milestone, None)
         self.assertEqual(self.plugin.logger.info.call_count, 0)
         self.assertEqual(self.plugin.logger.warning.call_count, 0)
@@ -285,9 +286,10 @@
     def test__set_milestone_on_task_milestone_already_set(self):
         """milestone is already set, should leave task untouched"""
         self.plugin.logger.info = MagicMock()
-        self.plugin.config = {"set_milestone": True, "default_milestone": "past"}
+        self.plugin.config = {"set_milestone": True,
+                              "default_milestone": "past"}
         self.plugin._set_milestone_on_task(
-            self.projects[0], self.bugs['1'].bug_tasks[0])
+            self.series[0], self.bugs['1'].bug_tasks[0])
         self.assertEqual(
             self.bugs['1'].bug_tasks[0].milestone, self.milestone_with_bug)
         self.assertIn("already has milestone",
@@ -298,7 +300,7 @@
         self.plugin.logger.info = MagicMock()
         self.plugin.config = {"set_milestone": True, "default_milestone": None}
         self.plugin._set_milestone_on_task(
-            self.projects[0], self.bugs['0'].bug_tasks[0])
+            self.series[0], self.bugs['0'].bug_tasks[0])
         self.assertEqual(
             self.bugs['0'].bug_tasks[0].milestone, self.milestone_future)
         self.assertEqual(self.plugin.logger.info.call_count, 1)