Merge lp:~dpb/tarmac/set-milestone-2 into lp:tarmac
- set-milestone-2
- Merge into main
Status: | Merged |
---|---|
Approved by: | dobey |
Approved revision: | 441 |
Merged at revision: | 432 |
Proposed branch: | lp:~dpb/tarmac/set-milestone-2 |
Merge into: | lp:tarmac |
Diff against target: |
477 lines (+381/-3) 5 files modified
docs/introduction.txt (+16/-0) tarmac/plugins/__init__.py (+18/-0) tarmac/plugins/bugresolver.py (+106/-0) tarmac/plugins/tests/test_bugresolver.py (+196/-3) tarmac/plugins/tests/test_tarmac_plugin.py (+45/-0) |
To merge this branch: | bzr merge lp:~dpb/tarmac/set-milestone-2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
dobey | Approve | ||
Chad Smith | Approve | ||
Данило Шеган (community) | Approve | ||
Review via email: mp+210888@code.launchpad.net |
Commit message
Add an option to bugresolver to set the milestone if not already set.
Description of the change
When resolving a bug, set the milestone if not already set.
Milestone is selected using the following algorithm:
1) nearest milestone with target date in the future
2) if no milestones are in the future, sort remaining open milestones lexically by name and pick the first
3) If all milestones have dates and we are past all dates, pick the newest of those.
4) override all that logic with a config setting of "default_milestone" which is the specific milestone name to use (which will even select a non-active milestone).
This behavior only engages if the config setting 'set_milestone = True" is present in your config file.
You can test with: trial -j4 tarmac (or ./run-tests)
Read "Hacking" for a small list of deps needed for testing.
David Britton (dpb) wrote : | # |
On Thu, Mar 13, 2014 at 07:42:35PM -0000, Rodney Dawes wrote:
> Review: Needs Fixing
>
> Can you please file bugs for new features, and link them? You'll also need to set the commit message.
Yes, but we want to do an internal review first, so I hadn't done
everything yet. Please feel free to ignore until we get there.
--
David Britton <email address hidden>
Данило Шеган (danilo) wrote : | # |
Hi David,
Thanks for taking on this. The branch looks pretty good, though, with
tarmac not being code-base I am familiar with, I'll try to infer some of
the policies from the surrounding code and make a few suggestions along
the way (mostly for code structure and tests).
The review is going to be long, but mostly mechanical/
I simply can't restrain myself suggesting a more optimal approach
sometimes, even though it probably makes no difference for small numbers
of milestones most everybody is going to have. That means that you need
not follow up on any of them :)
review needs-fixing
Cheers,
Danilo
> === modified file 'docs/introduct
> @@ -263,6 +263,22 @@
...
> + # the one that sorts least amoung them the target. If this
algorithm
s/amoung/among/
> === modified file 'tarmac/
> + def get_config(self, attribute, default, *args):
...
> + if hasattr(obj, "config"):
> + if hasattr(obj.config, attribute):
You can simplify this as
if hasattr(obj, "config") and hasattr(obj.config,
attribute):
to save one indentation level (Python will not evaluate right side of
"and" when the left one evaluates to False). Not a big deal here, but
can be handy when your ifs start getting deeply nested.
I am not sure this method really belongs as-is in the TarmacPlugin
either: it seems to be very generic (eg. no mention of "self" in the
body, and it's not used anywhere inside TarmacPlugin methods). At the
very least, it should be a function.
I don't see unit tests for this method either (and they should be pretty
simple to write).
> === modified file 'tarmac/
> + def _get_and_
> + """
> + Retrieve and parse configuration settings for this plugin.
> + Return as a dict structure.
> + """
> + set_milestone = self.get_
*args)
> + if set_milestone.
> + set_milestone = True
> + else:
> + set_milestone = False
> +
> + default = self.get_
> + if default is not None and not len(default):
> + default = None
Why check for len() here? I assume you wanted default == "", so I
suggest you be explicit. It'd also make the check simpler since it'd be
just
if default == "":
default = None
> + def _set_milestone_
...
I know you are testing this with test_run_
be good to have a unit test or two for this as well. I can see a few
cases here:
- config option not set, no-op on task milestone
- milestone already set, no-op on task milestone, warning logged
- config option set, milestone is being set, action logged
> + def _find_milestone
> + """
> + Return list of milestones in a project. Filter list by
active status.
> + If the config `default_milestone` is set filter by that
instead.
> + """
> + default = self.config[
> + milestones = []
> + for milestone in pro...
David Britton (dpb) wrote : | # |
On Wed, Mar 19, 2014 at 05:09:17PM -0000, Данило Шеган wrote:
> Review: Needs Fixing
Wow, thanks for the Review Danilo -- very helpful. I implemented most
of your suggestions, but left a couple on the table. I'll note below...
Please re-review when you get a chance.
> s/amoung/among/
Fixed.
>
> > === modified file 'tarmac/
> > + def get_config(self, attribute, default, *args):
> ...
> > + if hasattr(obj, "config"):
> > + if hasattr(obj.config, attribute):
>
> You can simplify this as
>
> if hasattr(obj, "config") and hasattr(obj.config, attribute):
Fixed.
>
> I am not sure this method really belongs as-is in the TarmacPlugin
> either: it seems to be very generic (eg. no mention of "self" in the
> body, and it's not used anywhere inside TarmacPlugin methods). At the
> very least, it should be a function.
Thanks -- I changed it to a @staticmethod. I'm willing to move it, but
I can't really find a good place. I saw this type of logic repeated in
many plugins, so I figured the plugins base class would be a good place
to start. I guess it can always be moved in the future. Let me know...
>
> I don't see unit tests for this method either (and they should be pretty
> simple to write).
Fixed.
>
> > === modified file 'tarmac/
> > + def _get_and_
> > + """
> > + Retrieve and parse configuration settings for this plugin.
> > + Return as a dict structure.
> > + """
> > + set_milestone = self.get_
> > + if set_milestone.
> > + set_milestone = True
> > + else:
> > + set_milestone = False
> > +
> > + default = self.get_
> > + if default is not None and not len(default):
> > + default = None
>
> Why check for len() here? I assume you wanted default == "", so I
> suggest you be explicit. It'd also make the check simpler since it'd be
> just
>
> if default == "":
> default = None
Fixed.
>
> > + def _set_milestone_
> ...
>
> I know you are testing this with test_run_
> be good to have a unit test or two for this as well. I can see a few
> cases here:
>
> - config option not set, no-op on task milestone
> - milestone already set, no-op on task milestone, warning logged
> - config option set, milestone is being set, action logged
Good catch. Fixed.
>
> > + def _find_milestone
> > + """
> > + Return list of milestones in a project. Filter list by
> active status.
> > + If the config `default_milestone` is set filter by that
> instead.
> > + """
> > + default = self.config[
> > + milestones = []
> > + for milestone in project.
>
> You can optimize by fetching only active_milestones.
Fixed. I didn't know about active_milestones. Indeed, that simplified
much of the code.
>
> > + if default is not None:
> > + ...
Данило Шеган (danilo) wrote : | # |
Hi David,
Thanks for bearing with me. Most everything looks good, but you named your new test file
test_
The rest is really good now, thanks.
review approve
> OK, I took your suggestion. Though I still like the previous one as I
> found it easier to follow. I was curious, so I timed some stuff:
>
> 1000 milestones, 10,000 iterstions:
>
> old: 46.1424999237
> new: 44.1123301983
>
> 1,000,000 milestones, 10 iterations:
>
> old: 57.2593920231
> new: 49.6416790485
>
> As expected, n log n nature of the sort is slowing things down when large
> amounts of records come into play. And... I think this is quickly
> entering a theoretical discussion. :)
Heh, indeed. Thanks for testing it out: the difference might rather amount to
Python memory management (you are still dealing with duplicated structures in
the original variant) since the loop is much more complicated (and thus
slower) in "my" proposal (and sorted() is heavily optimized in Python).
So, in the end, it was probably not worth it even for large sets of
milestones, but it was fun nevertheless :)
Chad Smith (chad.smith) wrote : | # |
[1] in test_bugresolver setUp:
Not that it'll take a long time to run setUp, but we should move self.now to the top and use self.now when creating all the self.milestone_* attributes so they are all based on the same now.
247 + self.now = datetime.utcnow()
[2]test_
+ """Test plug-in with the set_milestone setting runs correctly."""
[3] test_run_
277 + self.assertEqua
[4] In test__find_
now = self.milestone_
self.assertTrue(now , self.milestone_
milestone = self.plugin.
self.
[5] variable naming for in _get_and_
96 + default = self.get_
97 + if default == "":
98 + default = None
[6] docstring fix in _find_target_
3) the last milestone in the list (covers len()==1 case).
"the last" -> "the most recent"
Chad Smith (chad.smith) wrote : | # |
Ran through a few live trials of this with a development project and was able to validate lexical milestone setting, leaving existing milestones unchanged and assigning active milestones at the closest specific dates. Nice job +1!
David Britton (dpb) wrote : | # |
OK, I've deployed to our jenkins environment. We will get some use of it then submit upstream for inclusion.
David Britton (dpb) wrote : | # |
We've been running this now for 2 months, it's working well and is an optional feature. Asking for inclusion in upstream.
dobey (dobey) wrote : | # |
> We've been running this now for 2 months, it's working well and is an optional
> feature. Asking for inclusion in upstream.
OK. It'll probably be a couple weeks more before I can get to reviewing it, due to sprint and vacation. I'll try to review it as soon as I can though.
David Britton (dpb) wrote : | # |
Thanks!
On Fri, May 23, 2014 at 2:26 PM, Rodney Dawes <email address hidden>wrote:
> > We've been running this now for 2 months, it's working well and is an
> optional
> > feature. Asking for inclusion in upstream.
>
> OK. It'll probably be a couple weeks more before I can get to reviewing
> it, due to sprint and vacation. I'll try to review it as soon as I can
> though.
> --
>
> https:/
> You are the owner of lp:~davidpbritton/tarmac/set-milestone-2.
>
--
David Britton <email address hidden>
dobey (dobey) : | # |
Preview Diff
1 | === modified file 'docs/introduction.txt' |
2 | --- docs/introduction.txt 2014-02-09 23:06:33 +0000 |
3 | +++ docs/introduction.txt 2014-03-24 21:40:01 +0000 |
4 | @@ -263,6 +263,22 @@ |
5 | |
6 | bzr commit --fixes=lp:000000 |
7 | |
8 | +This plugin supports options: |
9 | + |
10 | + [lp:target_branch] |
11 | + # When setting `Fixed Committed`, set the milestone on the bug to the |
12 | + # closest upcoming milestone (based on the milestone "expected date") |
13 | + # If past all milestones, set to the newest. Milestones with no dates |
14 | + # set are assumed to be in the future and lexically sorted -- making |
15 | + # the one that sorts least among them the target. If this algorithm |
16 | + # doesn't work for you, see `default_milestone`. |
17 | + set_milestone = True |
18 | + |
19 | + # When setting `Fixed Committed`, set the milestone on the bug to this. |
20 | + # Requires `set_milestone = True`. If this doesn't match any of your |
21 | + # active milestone names, a warning message will be printed. |
22 | + default_milestone = 12.10 |
23 | + |
24 | |
25 | ======================== |
26 | Installing other plugins |
27 | |
28 | === modified file 'tarmac/plugins/__init__.py' |
29 | --- tarmac/plugins/__init__.py 2010-07-18 16:01:17 +0000 |
30 | +++ tarmac/plugins/__init__.py 2014-03-24 21:40:01 +0000 |
31 | @@ -29,3 +29,21 @@ |
32 | |
33 | def run(self, *args, **kwargs): |
34 | '''Run the hook.''' |
35 | + |
36 | + @staticmethod |
37 | + def get_config(attribute, default, *args): |
38 | + ''' |
39 | + Lookup configuration and return to caller. |
40 | + Return value of `default` if not found. Typically you would call |
41 | + in the following manner: |
42 | + |
43 | + value = get_config("my_config_setting", None, command, target) |
44 | + |
45 | + @param attribute: The attribute to lookup. |
46 | + @param default: The default value to use if nothing found. |
47 | + @param *args: objects to probe looking for the requested attribute. |
48 | + ''' |
49 | + for obj in args: |
50 | + if hasattr(obj, "config") and hasattr(obj.config, attribute): |
51 | + return getattr(obj.config, attribute) |
52 | + return default |
53 | |
54 | === modified file 'tarmac/plugins/bugresolver.py' |
55 | --- tarmac/plugins/bugresolver.py 2010-10-05 18:12:42 +0000 |
56 | +++ tarmac/plugins/bugresolver.py 2014-03-24 21:40:01 +0000 |
57 | @@ -16,6 +16,7 @@ |
58 | |
59 | from tarmac.hooks import tarmac_hooks |
60 | from tarmac.plugins import TarmacPlugin |
61 | +from datetime import datetime |
62 | |
63 | |
64 | class BugResolver(TarmacPlugin): |
65 | @@ -27,6 +28,9 @@ |
66 | if not fixed_bugs: |
67 | return |
68 | |
69 | + # Retrieve configuration |
70 | + self.config = self._get_and_parse_config(target, command) |
71 | + |
72 | project = target.lp_branch.project |
73 | try: |
74 | series_name = target.lp_branch.bzr_identity.split('/')[1] |
75 | @@ -53,10 +57,112 @@ |
76 | |
77 | if task: |
78 | task.status = u'Fix Committed' |
79 | + self._set_milestone_on_task(project, task) |
80 | task.lp_save() |
81 | else: |
82 | self.logger.info('Target %s/%s not found in bug #%s.', |
83 | project.name, series.name, bug_id) |
84 | |
85 | + def _get_and_parse_config(self, *args): |
86 | + """ |
87 | + Retrieve and parse configuration settings for this plugin. |
88 | + Return as a dict structure. |
89 | + """ |
90 | + set_milestone = self.get_config("set_milestone", "False", *args) |
91 | + if set_milestone.lower() == "true" or set_milestone == "1": |
92 | + set_milestone = True |
93 | + else: |
94 | + set_milestone = False |
95 | + |
96 | + default_milestone = self.get_config("default_milestone", None, *args) |
97 | + if default_milestone == "": |
98 | + default_milestone = None |
99 | + |
100 | + return { |
101 | + "set_milestone": set_milestone, |
102 | + "default_milestone": default_milestone} |
103 | + |
104 | + def _set_milestone_on_task(self, project, task): |
105 | + """ |
106 | + Attempt to auto-determine the milestone to set, and set the milestone |
107 | + of the given task. If the task already has a milestone set => noop. |
108 | + Only processed if config setting `set_milestone` == True. |
109 | + """ |
110 | + if not self.config["set_milestone"]: |
111 | + return |
112 | + task_milestone = task.milestone |
113 | + if task_milestone is not None: |
114 | + self.logger.info( |
115 | + "Task: %s/%s already has milestone: %s set, skipping" % ( |
116 | + task.bug.id, task.bug_target_name, task_milestone.name)) |
117 | + return |
118 | + now = datetime.utcnow() |
119 | + target_milestone = self._find_target_milestone(project, now) |
120 | + self.logger.info("%s/%s: Setting Milestone: %s", |
121 | + task.bug.id, task.bug_target_name, target_milestone) |
122 | + task.milestone = target_milestone |
123 | + |
124 | + def _find_milestones(self, project): |
125 | + """ |
126 | + Return list of active milestones in a project. If the config |
127 | + `default_milestone` is set filter by that. If not found, an |
128 | + empty list will be returned. |
129 | + """ |
130 | + default = self.config["default_milestone"] |
131 | + if default is None: |
132 | + return list(project.active_milestones) |
133 | + for milestone in project.active_milestones: |
134 | + if milestone.name == default: |
135 | + return [milestone] |
136 | + self.logger.warning("Default Milestone not found: %s" % default) |
137 | + return [] |
138 | + |
139 | + def _find_target_milestone(self, project, now): |
140 | + """ |
141 | + Find a target milestone when resolving a bug task. |
142 | + |
143 | + Compare the selected datetime `now` to the list of milestones. |
144 | + Return the milestone where `targeted_date` is newer than the given |
145 | + datetime. If the given time is greater than all open milestones: |
146 | + target to the newest milestone in the list. |
147 | + |
148 | + In this algorithm, milestones without targeted dates appear lexically |
149 | + sorted at the end of the list. So the lowest sorting one will get |
150 | + chosen if all milestones with dates attached are exhausted. |
151 | + |
152 | + In other words, pick one of the milestones for the target. Preference: |
153 | + 1) closest milestone (by date) in the future |
154 | + 2) least lexically sorting milestone (by name) |
155 | + 3) the last milestone in the list (covers len()==1 case). |
156 | + """ |
157 | + milestones = self._find_milestones(project) |
158 | + earliest_after = latest_before = untargeted = None |
159 | + for milestone in milestones: |
160 | + if milestone.date_targeted is None: |
161 | + if untargeted is not None: |
162 | + if milestone.name < untargeted.name: |
163 | + untargeted = milestone |
164 | + else: |
165 | + untargeted = milestone |
166 | + elif milestone.date_targeted > now: |
167 | + if earliest_after is not None: |
168 | + if earliest_after.date_targeted > milestone.date_targeted: |
169 | + earliest_after = milestone |
170 | + else: |
171 | + earliest_after = milestone |
172 | + elif milestone.date_targeted < now: |
173 | + if latest_before is not None: |
174 | + if latest_before.date_targeted < milestone.date_targeted: |
175 | + latest_before = milestone |
176 | + else: |
177 | + latest_before = milestone |
178 | + |
179 | + if earliest_after is not None: |
180 | + return earliest_after |
181 | + elif untargeted is not None: |
182 | + return untargeted |
183 | + else: |
184 | + return latest_before |
185 | + |
186 | |
187 | tarmac_hooks['tarmac_post_commit'].hook(BugResolver(), 'Bug resolver') |
188 | |
189 | === modified file 'tarmac/plugins/tests/test_bugresolver.py' |
190 | --- tarmac/plugins/tests/test_bugresolver.py 2013-10-31 19:09:08 +0000 |
191 | +++ tarmac/plugins/tests/test_bugresolver.py 2014-03-24 21:40:01 +0000 |
192 | @@ -18,6 +18,8 @@ |
193 | from tarmac.plugins.bugresolver import BugResolver |
194 | from tarmac.tests import TarmacTestCase |
195 | from tarmac.tests import Thing |
196 | +from datetime import datetime, timedelta |
197 | +from mock import MagicMock |
198 | |
199 | |
200 | class BugResolverTests(TarmacTestCase): |
201 | @@ -26,8 +28,28 @@ |
202 | def setUp(self): |
203 | """Set up data for the tests.""" |
204 | super(BugResolverTests, self).setUp() |
205 | + self.now = datetime.utcnow() |
206 | self.proposal = Thing() |
207 | self.plugin = BugResolver() |
208 | + self.plugin.config = { |
209 | + "set_milestone": "False", |
210 | + "default_milestone": None} |
211 | + self.milestone_untargeted_a = Thing(name="a", date_targeted=None) |
212 | + self.milestone_past = Thing( |
213 | + name="past", |
214 | + date_targeted=self.now - timedelta(weeks=2)) |
215 | + self.milestone_future = Thing( |
216 | + name="future", |
217 | + date_targeted=self.now + timedelta(weeks=2)) |
218 | + self.milestone_far_future = Thing( |
219 | + name="far_future", |
220 | + date_targeted=self.now + timedelta(weeks=6)) |
221 | + self.milestone_untargeted_b = Thing(name="b", date_targeted=None) |
222 | + self.milestone_untargeted_c = Thing(name="c", date_targeted=None) |
223 | + self.milestone_with_bug = Thing( |
224 | + name="with_bug", |
225 | + date_targeted=self.now - timedelta(weeks=3), |
226 | + bug=Thing(id=12345), bug_target_name="foo_project") |
227 | self.series = [Thing(name='trunk'), |
228 | Thing(name='stable')] |
229 | self.projects = [Thing(name='target', |
230 | @@ -40,12 +62,29 @@ |
231 | self.targets[0] = self.projects[0] |
232 | self.bugs = {'0': Thing( |
233 | bug_tasks=[Thing(target=self.targets[0], status=u'In Progress', |
234 | - lp_save=self.lp_save), |
235 | + lp_save=self.lp_save, milestone=None, |
236 | + bug=Thing(id="0"), |
237 | + bug_target_name=self.targets[0].name), |
238 | Thing(target=self.targets[2], status=u'Incomplete', |
239 | - lp_save=self.lp_save)]), |
240 | + lp_save=self.lp_save, milestone=None, |
241 | + bug=Thing(id="0"), |
242 | + bug_target_name=self.targets[2].name)]), |
243 | '1': Thing( |
244 | bug_tasks=[Thing(target=self.targets[1], status=u'Confirmed', |
245 | - lp_save=self.lp_save)])} |
246 | + lp_save=self.lp_save, |
247 | + milestone=self.milestone_with_bug, |
248 | + bug=Thing(id="1"), |
249 | + bug_target_name=self.targets[2].name)])} |
250 | + # Insert out of order to make sure they sort correctly. |
251 | + self.milestones = [ |
252 | + self.milestone_far_future, self.milestone_with_bug, |
253 | + self.milestone_past, self.milestone_future] |
254 | + self.milestones_extended = [ |
255 | + self.milestone_untargeted_c, self.milestone_untargeted_b, |
256 | + self.milestone_untargeted_a] |
257 | + self.milestones_extended.extend(self.milestones) |
258 | + self.projects[0].active_milestones = self.milestones |
259 | + self.projects[1].active_milestones = self.milestones_extended |
260 | |
261 | def getSeries(self, name=None): |
262 | """Faux getSeries for testing.""" |
263 | @@ -71,6 +110,27 @@ |
264 | self.assertEqual(self.bugs['0'].bug_tasks[1].status, u'Incomplete') |
265 | self.assertEqual(self.bugs['1'].bug_tasks[0].status, u'Confirmed') |
266 | |
267 | + def test_run_with_set_milestone(self): |
268 | + """ |
269 | + Test plug-in with the set_milestone config setting = true. Will |
270 | + auto-resolve milestone using algorithm in find_target_milestone |
271 | + and set. Test that bug0/task0 and bug1/task0 get the correct |
272 | + milestone set. bug0/task1 is incomplete so should not be touched. |
273 | + """ |
274 | + target = Thing(fixed_bugs=self.bugs.keys(), |
275 | + lp_branch=Thing(project=self.projects[0], |
276 | + bzr_identity='lp:target'), |
277 | + config=Thing(set_milestone="true")) |
278 | + launchpad = Thing(bugs=self.bugs) |
279 | + command = Thing(launchpad=launchpad) |
280 | + self.plugin.run(command=command, target=target, source=None, |
281 | + proposal=self.proposal) |
282 | + self.assertEqual(self.bugs['0'].bug_tasks[0].milestone, |
283 | + self.milestone_future) |
284 | + self.assertEqual(self.bugs['1'].bug_tasks[0].milestone, |
285 | + self.milestone_with_bug) |
286 | + self.assertIsNone(self.bugs['0'].bug_tasks[1].milestone) |
287 | + |
288 | def test_run_with_no_bugs(self): |
289 | """Test that bug resolution for no bugs does nothing.""" |
290 | target = Thing(fixed_bugs=None, |
291 | @@ -109,3 +169,136 @@ |
292 | self.assertEqual(self.bugs['0'].bug_tasks[0].status, u'In Progress') |
293 | self.assertEqual(self.bugs['0'].bug_tasks[1].status, u'Incomplete') |
294 | self.assertEqual(self.bugs['1'].bug_tasks[0].status, u'Confirmed') |
295 | + |
296 | + def test__find_target_milestone_older(self): |
297 | + """Dates before all milestones return the oldest milestone.""" |
298 | + milestone = self.plugin._find_target_milestone( |
299 | + self.projects[0], |
300 | + self.milestone_past.date_targeted - timedelta(weeks=1)) |
301 | + self.assertEqual(milestone, self.milestone_past) |
302 | + |
303 | + def test__find_target_milestone_between(self): |
304 | + """Test that dates between milestones return the closest newest.""" |
305 | + milestone = self.plugin._find_target_milestone( |
306 | + self.projects[1], |
307 | + self.milestone_past.date_targeted + timedelta(weeks=1)) |
308 | + self.assertTrue(self.now < self.milestone_future.date_targeted) |
309 | + self.assertTrue(self.now > self.milestone_past.date_targeted) |
310 | + self.assertEqual(milestone, self.milestone_future) |
311 | + |
312 | + def test__find_target_milestone_newer(self): |
313 | + """Test that dates after all milestones return the newest.""" |
314 | + milestone = self.plugin._find_target_milestone( |
315 | + self.projects[0], |
316 | + self.milestone_far_future.date_targeted + timedelta(weeks=1)) |
317 | + self.assertEqual(milestone, self.milestone_far_future) |
318 | + |
319 | + def test__find_target_milestone_lexical_sort_past_dates(self): |
320 | + """Dates after milestones return the least sorted no-expected-date.""" |
321 | + milestone = self.plugin._find_target_milestone( |
322 | + self.projects[1], |
323 | + self.milestone_far_future.date_targeted + timedelta(weeks=1)) |
324 | + self.assertEqual(milestone, self.milestone_untargeted_a) |
325 | + |
326 | + def test__find_target_milestone_with_default(self): |
327 | + """Test that specifying a default gets a specific milestone.""" |
328 | + self.plugin.config["default_milestone"] = "c" |
329 | + milestone = self.plugin._find_target_milestone( |
330 | + self.projects[1], |
331 | + self.milestone_far_future.date_targeted + timedelta(weeks=1)) |
332 | + self.assertEqual(milestone, self.milestone_untargeted_c) |
333 | + |
334 | + def test__find_milestone_positive(self): |
335 | + """Given a project, the list of milestones is returned.""" |
336 | + milestones = self.plugin._find_milestones(self.projects[1]) |
337 | + self.assertEqual(len(milestones), 7) |
338 | + |
339 | + def test__find_milestone_negative(self): |
340 | + """Given a project with no milestones, _find_milestone handles it""" |
341 | + milestones = self.plugin._find_milestones(Thing(active_milestones=[])) |
342 | + self.assertEqual(len(milestones), 0) |
343 | + |
344 | + def test__find_milestone_specific_negative(self): |
345 | + """Find a secific milestone that isn't there, check for log""" |
346 | + self.plugin.logger.warning = MagicMock() |
347 | + self.plugin.config["default_milestone"] = "FOO" |
348 | + milestones = self.plugin._find_milestones(self.projects[0]) |
349 | + self.assertEqual(len(milestones), 0) |
350 | + self.assertEqual(self.plugin.logger.warning.call_count, 1) |
351 | + |
352 | + def test__find_milestone_no_dates(self): |
353 | + """Find a specific milestones without a targeted date""" |
354 | + self.plugin.config["default_milestone"] = "b" |
355 | + milestones = self.plugin._find_milestones(self.projects[1]) |
356 | + self.assertEqual(len(milestones), 1) |
357 | + self.assertEqual(milestones[0], self.milestone_untargeted_b) |
358 | + |
359 | + def test__get_and_parse_config_set_milestone_true_upper(self): |
360 | + """Test config parsing - set_milestone: True.""" |
361 | + config = self.plugin._get_and_parse_config( |
362 | + Thing(config=Thing(set_milestone="True"))) |
363 | + self.assertEqual(config["set_milestone"], True) |
364 | + |
365 | + def test__get_and_parse_config_set_milestone_one(self): |
366 | + """Test config parsing - set_milestone: 1.""" |
367 | + config = self.plugin._get_and_parse_config( |
368 | + Thing(config=Thing(set_milestone="1"))) |
369 | + self.assertEqual(config["set_milestone"], True) |
370 | + |
371 | + def test__get_and_parse_config_set_milestone_true_lower(self): |
372 | + """Test config parsing - set_milestone: true.""" |
373 | + config = self.plugin._get_and_parse_config( |
374 | + Thing(config=Thing(set_milestone="true"))) |
375 | + self.assertEqual(config["set_milestone"], True) |
376 | + |
377 | + def test__get_and_parse_config_default_milestone_A(self): |
378 | + """Test config parsing - default_milestone: A.""" |
379 | + config = self.plugin._get_and_parse_config( |
380 | + Thing(config=Thing(default_milestone="A"))) |
381 | + self.assertEqual(config["default_milestone"], "A") |
382 | + |
383 | + def test__get_and_parse_config_default_milestone_none(self): |
384 | + """Test config parsing - default_milestone: .""" |
385 | + config = self.plugin._get_and_parse_config( |
386 | + Thing(config=Thing(default_milestone=""))) |
387 | + self.assertEqual(config["default_milestone"], None) |
388 | + |
389 | + def test__get_and_parse_config_default_milestone_default(self): |
390 | + """Test config parsing - defaults.""" |
391 | + config = self.plugin._get_and_parse_config( |
392 | + Thing(config=Thing())) |
393 | + self.assertEqual(config["set_milestone"], False) |
394 | + self.assertEqual(config["default_milestone"], None) |
395 | + |
396 | + def test__set_milestone_on_task_config_not_set(self): |
397 | + """config option not set, no-op""" |
398 | + self.plugin.logger.info = MagicMock() |
399 | + self.plugin.logger.warning = MagicMock() |
400 | + self.plugin.config = { |
401 | + "set_milestone": False, "default_milestone": None} |
402 | + self.plugin._set_milestone_on_task( |
403 | + self.projects[0], self.bugs['0'].bug_tasks[0]) |
404 | + self.assertEqual(self.bugs['0'].bug_tasks[0].milestone, None) |
405 | + self.assertEqual(self.plugin.logger.info.call_count, 0) |
406 | + self.assertEqual(self.plugin.logger.warning.call_count, 0) |
407 | + |
408 | + def test__set_milestone_on_task_milestone_already_set(self): |
409 | + """milestone is already set, should leave task untouched""" |
410 | + self.plugin.logger.info = MagicMock() |
411 | + self.plugin.config = {"set_milestone": True, "default_milestone": "past"} |
412 | + self.plugin._set_milestone_on_task( |
413 | + self.projects[0], self.bugs['1'].bug_tasks[0]) |
414 | + self.assertEqual( |
415 | + self.bugs['1'].bug_tasks[0].milestone, self.milestone_with_bug) |
416 | + self.assertIn("already has milestone", |
417 | + self.plugin.logger.info.call_args[0][0]) |
418 | + |
419 | + def test__set_milestone_on_task_config_set(self): |
420 | + """config option set, milestone is being set, action logged""" |
421 | + self.plugin.logger.info = MagicMock() |
422 | + self.plugin.config = {"set_milestone": True, "default_milestone": None} |
423 | + self.plugin._set_milestone_on_task( |
424 | + self.projects[0], self.bugs['0'].bug_tasks[0]) |
425 | + self.assertEqual( |
426 | + self.bugs['0'].bug_tasks[0].milestone, self.milestone_future) |
427 | + self.assertEqual(self.plugin.logger.info.call_count, 1) |
428 | |
429 | === added file 'tarmac/plugins/tests/test_tarmac_plugin.py' |
430 | --- tarmac/plugins/tests/test_tarmac_plugin.py 1970-01-01 00:00:00 +0000 |
431 | +++ tarmac/plugins/tests/test_tarmac_plugin.py 2014-03-24 21:40:01 +0000 |
432 | @@ -0,0 +1,45 @@ |
433 | +# Copyright 2014 Canonical, Ltd. |
434 | +# |
435 | +# This file is part of Tarmac. |
436 | +# |
437 | +# Tarmac is free software: you can redistribute it and/or modify |
438 | +# it under the terms of the GNU General Public License version 3 as |
439 | +# published by the Free Software Foundation. |
440 | +# |
441 | +# Tarmac is distributed in the hope that it will be useful, |
442 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
443 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
444 | +# GNU General Public License for more details. |
445 | +# |
446 | +# You should have received a copy of the GNU General Public License |
447 | +# along with Tarmac. If not, see <http://www.gnu.org/licenses/>. |
448 | +"""Tests for base/abstract TarmacPlugin class.""" |
449 | + |
450 | +from tarmac.plugins import TarmacPlugin |
451 | +from tarmac.tests import Thing, TarmacTestCase |
452 | + |
453 | +class TestTarmacPlugin(TarmacTestCase): |
454 | + """Test the tarmacplugin abstract class.""" |
455 | + |
456 | + def test_get_config_missing(self): |
457 | + """get_config returns default values.""" |
458 | + value = TarmacPlugin.get_config("foo", "default", Thing()) |
459 | + self.assertEqual(value, "default") |
460 | + value = TarmacPlugin.get_config("foo", None, Thing(config=None)) |
461 | + self.assertEqual(value, None) |
462 | + |
463 | + def test_get_config_basic(self): |
464 | + """get_config returns values.""" |
465 | + value = TarmacPlugin.get_config( |
466 | + "foo", "default", Thing(config=Thing(foo="bar"))) |
467 | + self.assertEqual(value, "bar") |
468 | + |
469 | + def test_get_config_multiple(self): |
470 | + """get_config returns values from multiple objects.""" |
471 | + value = TarmacPlugin.get_config( |
472 | + "foo", "default", |
473 | + Thing(config=Thing(bar="bar")), |
474 | + Thing(config=Thing(foo="first")), |
475 | + Thing(config=Thing(foo="second"))) |
476 | + self.assertEqual(value, "first") |
477 | + |
Can you please file bugs for new features, and link them? You'll also need to set the commit message.