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

Proposed by Adam Collard
Status: Rejected
Rejected by: Adam Collard
Proposed branch: lp:~adam-collard/tarmac/milestone-filter-by-series
Merge into: lp:tarmac
Diff against target: 208 lines (+36/-33)
2 files modified
tarmac/plugins/bugresolver.py (+13/-12)
tarmac/plugins/tests/test_bugresolver.py (+23/-21)
To merge this branch: bzr merge lp:~adam-collard/tarmac/milestone-filter-by-series
Reviewer Review Type Date Requested Status
dobey Needs Fixing
Chris Glass (community) Approve
Данило Шеган (community) Approve
Review via email: mp+278946@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) wrote :

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
Revision history for this message
Chris Glass (tribaal) wrote :

+1 !

review: Approve
435. By Adam Collard

No need to walk next_collection links

Revision history for this message
dobey (dobey) wrote :
Download full text (10.2 KiB)

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

No need to walk next_collection links

434. By Adam Collard

fix lint

433. By Adam Collard

Filter by series

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tarmac/plugins/bugresolver.py'
2--- tarmac/plugins/bugresolver.py 2014-03-24 21:38:36 +0000
3+++ tarmac/plugins/bugresolver.py 2015-12-02 15:37:05 +0000
4@@ -57,7 +57,7 @@
5
6 if task:
7 task.status = u'Fix Committed'
8- self._set_milestone_on_task(project, task)
9+ self._set_milestone_on_task(series, task)
10 task.lp_save()
11 else:
12 self.logger.info('Target %s/%s not found in bug #%s.',
13@@ -82,7 +82,7 @@
14 "set_milestone": set_milestone,
15 "default_milestone": default_milestone}
16
17- def _set_milestone_on_task(self, project, task):
18+ def _set_milestone_on_task(self, series, task):
19 """
20 Attempt to auto-determine the milestone to set, and set the milestone
21 of the given task. If the task already has a milestone set => noop.
22@@ -97,27 +97,28 @@
23 task.bug.id, task.bug_target_name, task_milestone.name))
24 return
25 now = datetime.utcnow()
26- target_milestone = self._find_target_milestone(project, now)
27+ target_milestone = self._find_target_milestone(series, now)
28 self.logger.info("%s/%s: Setting Milestone: %s",
29- task.bug.id, task.bug_target_name, target_milestone)
30+ task.bug.id, task.bug_target_name, target_milestone)
31 task.milestone = target_milestone
32
33- def _find_milestones(self, project):
34+ def _find_milestones(self, series):
35 """
36- Return list of active milestones in a project. If the config
37- `default_milestone` is set filter by that. If not found, an
38- empty list will be returned.
39+ Return iterable of active milestones in a series. If
40+ the config `default_milestone` is set filter by that. If not
41+ found, an empty list will be returned.
42 """
43 default = self.config["default_milestone"]
44+ milestones = series.active_milestones
45 if default is None:
46- return list(project.active_milestones)
47- for milestone in project.active_milestones:
48+ return milestones
49+ for milestone in milestones:
50 if milestone.name == default:
51 return [milestone]
52 self.logger.warning("Default Milestone not found: %s" % default)
53 return []
54
55- def _find_target_milestone(self, project, now):
56+ def _find_target_milestone(self, series, now):
57 """
58 Find a target milestone when resolving a bug task.
59
60@@ -135,7 +136,7 @@
61 2) least lexically sorting milestone (by name)
62 3) the last milestone in the list (covers len()==1 case).
63 """
64- milestones = self._find_milestones(project)
65+ milestones = self._find_milestones(series)
66 earliest_after = latest_before = untargeted = None
67 for milestone in milestones:
68 if milestone.date_targeted is None:
69
70=== modified file 'tarmac/plugins/tests/test_bugresolver.py'
71--- tarmac/plugins/tests/test_bugresolver.py 2014-03-24 21:38:36 +0000
72+++ tarmac/plugins/tests/test_bugresolver.py 2015-12-02 15:37:05 +0000
73@@ -76,15 +76,15 @@
74 bug=Thing(id="1"),
75 bug_target_name=self.targets[2].name)])}
76 # Insert out of order to make sure they sort correctly.
77- self.milestones = [
78+ self.milestones = Thing(total_size=4, entries=[
79 self.milestone_far_future, self.milestone_with_bug,
80- self.milestone_past, self.milestone_future]
81- self.milestones_extended = [
82+ self.milestone_past, self.milestone_future])
83+ self.milestones_extended = Thing(total_size=3, entries=[
84 self.milestone_untargeted_c, self.milestone_untargeted_b,
85- self.milestone_untargeted_a]
86- self.milestones_extended.extend(self.milestones)
87- self.projects[0].active_milestones = self.milestones
88- self.projects[1].active_milestones = self.milestones_extended
89+ self.milestone_untargeted_a])
90+ self.milestones_extended.entries.extend(self.milestones.entries)
91+ self.series[0].active_milestones = self.milestones
92+ self.series[1].active_milestones = self.milestones_extended
93
94 def getSeries(self, name=None):
95 """Faux getSeries for testing."""
96@@ -124,7 +124,7 @@
97 launchpad = Thing(bugs=self.bugs)
98 command = Thing(launchpad=launchpad)
99 self.plugin.run(command=command, target=target, source=None,
100- proposal=self.proposal)
101+ proposal=self.proposal)
102 self.assertEqual(self.bugs['0'].bug_tasks[0].milestone,
103 self.milestone_future)
104 self.assertEqual(self.bugs['1'].bug_tasks[0].milestone,
105@@ -173,14 +173,14 @@
106 def test__find_target_milestone_older(self):
107 """Dates before all milestones return the oldest milestone."""
108 milestone = self.plugin._find_target_milestone(
109- self.projects[0],
110+ self.series[0],
111 self.milestone_past.date_targeted - timedelta(weeks=1))
112 self.assertEqual(milestone, self.milestone_past)
113
114 def test__find_target_milestone_between(self):
115 """Test that dates between milestones return the closest newest."""
116 milestone = self.plugin._find_target_milestone(
117- self.projects[1],
118+ self.series[1],
119 self.milestone_past.date_targeted + timedelta(weeks=1))
120 self.assertTrue(self.now < self.milestone_future.date_targeted)
121 self.assertTrue(self.now > self.milestone_past.date_targeted)
122@@ -189,14 +189,14 @@
123 def test__find_target_milestone_newer(self):
124 """Test that dates after all milestones return the newest."""
125 milestone = self.plugin._find_target_milestone(
126- self.projects[0],
127+ self.series[0],
128 self.milestone_far_future.date_targeted + timedelta(weeks=1))
129 self.assertEqual(milestone, self.milestone_far_future)
130
131 def test__find_target_milestone_lexical_sort_past_dates(self):
132 """Dates after milestones return the least sorted no-expected-date."""
133 milestone = self.plugin._find_target_milestone(
134- self.projects[1],
135+ self.series[1],
136 self.milestone_far_future.date_targeted + timedelta(weeks=1))
137 self.assertEqual(milestone, self.milestone_untargeted_a)
138
139@@ -204,32 +204,33 @@
140 """Test that specifying a default gets a specific milestone."""
141 self.plugin.config["default_milestone"] = "c"
142 milestone = self.plugin._find_target_milestone(
143- self.projects[1],
144+ self.series[1],
145 self.milestone_far_future.date_targeted + timedelta(weeks=1))
146 self.assertEqual(milestone, self.milestone_untargeted_c)
147
148 def test__find_milestone_positive(self):
149 """Given a project, the list of milestones is returned."""
150- milestones = self.plugin._find_milestones(self.projects[1])
151+ milestones = list(self.plugin._find_milestones(self.series[1]))
152 self.assertEqual(len(milestones), 7)
153
154 def test__find_milestone_negative(self):
155 """Given a project with no milestones, _find_milestone handles it"""
156- milestones = self.plugin._find_milestones(Thing(active_milestones=[]))
157+ milestones = list(self.plugin._find_milestones(
158+ Thing(active_milestones=Thing(total_size=0, entries=[]))))
159 self.assertEqual(len(milestones), 0)
160
161 def test__find_milestone_specific_negative(self):
162 """Find a secific milestone that isn't there, check for log"""
163 self.plugin.logger.warning = MagicMock()
164 self.plugin.config["default_milestone"] = "FOO"
165- milestones = self.plugin._find_milestones(self.projects[0])
166+ milestones = list(self.plugin._find_milestones(self.series[0]))
167 self.assertEqual(len(milestones), 0)
168 self.assertEqual(self.plugin.logger.warning.call_count, 1)
169
170 def test__find_milestone_no_dates(self):
171 """Find a specific milestones without a targeted date"""
172 self.plugin.config["default_milestone"] = "b"
173- milestones = self.plugin._find_milestones(self.projects[1])
174+ milestones = list(self.plugin._find_milestones(self.series[1]))
175 self.assertEqual(len(milestones), 1)
176 self.assertEqual(milestones[0], self.milestone_untargeted_b)
177
178@@ -277,7 +278,7 @@
179 self.plugin.config = {
180 "set_milestone": False, "default_milestone": None}
181 self.plugin._set_milestone_on_task(
182- self.projects[0], self.bugs['0'].bug_tasks[0])
183+ self.series[0], self.bugs['0'].bug_tasks[0])
184 self.assertEqual(self.bugs['0'].bug_tasks[0].milestone, None)
185 self.assertEqual(self.plugin.logger.info.call_count, 0)
186 self.assertEqual(self.plugin.logger.warning.call_count, 0)
187@@ -285,9 +286,10 @@
188 def test__set_milestone_on_task_milestone_already_set(self):
189 """milestone is already set, should leave task untouched"""
190 self.plugin.logger.info = MagicMock()
191- self.plugin.config = {"set_milestone": True, "default_milestone": "past"}
192+ self.plugin.config = {"set_milestone": True,
193+ "default_milestone": "past"}
194 self.plugin._set_milestone_on_task(
195- self.projects[0], self.bugs['1'].bug_tasks[0])
196+ self.series[0], self.bugs['1'].bug_tasks[0])
197 self.assertEqual(
198 self.bugs['1'].bug_tasks[0].milestone, self.milestone_with_bug)
199 self.assertIn("already has milestone",
200@@ -298,7 +300,7 @@
201 self.plugin.logger.info = MagicMock()
202 self.plugin.config = {"set_milestone": True, "default_milestone": None}
203 self.plugin._set_milestone_on_task(
204- self.projects[0], self.bugs['0'].bug_tasks[0])
205+ self.series[0], self.bugs['0'].bug_tasks[0])
206 self.assertEqual(
207 self.bugs['0'].bug_tasks[0].milestone, self.milestone_future)
208 self.assertEqual(self.plugin.logger.info.call_count, 1)

Subscribers

People subscribed via source and target branches