Merge lp:~chad.smith/lp2kanban/handle-multitask-with-no-linked-branches into lp:lp2kanban

Proposed by Chad Smith on 2016-04-15
Status: Merged
Merged at revision: 137
Proposed branch: lp:~chad.smith/lp2kanban/handle-multitask-with-no-linked-branches
Merge into: lp:lp2kanban
Diff against target: 273 lines (+87/-44)
3 files modified
src/lp2kanban/bugs2cards.py (+13/-5)
src/lp2kanban/kanban.py (+2/-0)
src/lp2kanban/tests/test_bugs2cards.py (+72/-39)
To merge this branch: bzr merge lp:~chad.smith/lp2kanban/handle-multitask-with-no-linked-branches
Reviewer Review Type Date Requested Status
Simon Poirier 2016-04-15 Approve on 2016-04-19
Free Ekanayaka 2016-04-15 Approve on 2016-04-18
Review via email: mp+292028@code.launchpad.net

Description of the change

Fix a corner case in lp2kanban where bug cards with multiple tasks were being left in CODING status, because get_bug_status didn't account for bugs fixed as trivials (without linked branches).

It also fixes our ignore series tasks logic to avoid tracking series task target status in favor of tracking only the trunk target status.

This branch handles cases specifically like this one:
Specific to bugs like: https://bugs.launchpad.net/landscape/+bug/1565839

In the above bug we now ignore task status of Fix Released and track only the trunk status of Fix Committed. This now pushes the card to the deploy lanes (QA::TODO), properly handling the fact that the bug has no branches linked because it was "trivialed".

To test:

# Either make this project
make configs
make credentials
make
make check
./bin/py ./src/lp2kanban/bugs2cards.py -c configs/sync.ini -b "Landscape 2016"

# Or Hijack jenkins kanban sync job and point the Repository URL to lp:~chad.smith/lp2kanban/handle-multitask-with-no-linked-branches instead of lp:lp2kanban

https://ci.lscape.net/job/kanban-sync/configure

# Or stop the jenkins kanban-sync job and patch the existing checkout (which is refreshed each run anyway)
ssh <email address hidden>; sudo -u jenkins bash; cd /var/lib/jenkins/workspaces/kanban-sync; bzr merge lp:~chad.smith/lp2kanban/handle-multitask-with-no-linked-branches; ./bin/py -c configs/sync.ini -b "Landscape 2016"

To post a comment you must log in.
Free Ekanayaka (free.ekanayaka) wrote :

LGTM, +1

review: Approve
Simon Poirier (simpoir) wrote :

+1 looks good. I was about to complain about PEP8 but I noticed the whole repo is already a mess in that regard.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lp2kanban/bugs2cards.py'
2--- src/lp2kanban/bugs2cards.py 2016-04-13 16:06:23 +0000
3+++ src/lp2kanban/bugs2cards.py 2016-04-15 17:27:22 +0000
4@@ -10,6 +10,7 @@
5 )
6 from lp2kanban.kanban import (
7 BRANCH_MP_REGEX,
8+ RESOURCE_TYPE_LINK_SERIES,
9 LeankitKanban,
10 LeankitTaskBoard,
11 Record,
12@@ -451,7 +452,7 @@
13 # status of None means card needs not to be moved.
14 status = None
15 branch_card_status = None
16- if branch_info:
17+ if branch_info.status:
18 if branch_info.status == 'In Progress':
19 branch_card_status = CardStatus.CODING
20 elif branch_info.status == 'In Review':
21@@ -466,7 +467,7 @@
22 status = CardStatus.NEW
23 elif bug_status in IN_PROGRESS_BUG_STATUSES:
24 status = CardStatus.CODING
25- if branch_info.status == 'Merged':
26+ if branch_card_status in (None, CardStatus.LANDING):
27 if bug_status == 'Fix Committed':
28 if 'qa-needstesting' in bug_tags:
29 status = CardStatus.QA
30@@ -475,10 +476,11 @@
31 status = CardStatus.DOWNTIME_DEPLOY
32 else:
33 status = CardStatus.DEPLOY
34- else:
35+ elif branch_card_status is not None:
36 status = CardStatus.LANDING
37 elif branch_card_status:
38 return branch_card_status
39+
40 elif bug_status in DONE_FIX_BUG_STATUSES:
41 status = CardStatus.DONE_FIX
42 elif bug_status in DONE_NOFIX_BUG_STATUSES:
43@@ -492,9 +494,15 @@
44 assignees = []
45 tasks = lp_bug.bug_tasks_collection
46 for task in tasks:
47+ # Ignore bug task status for series targets
48+ if (task.target.resource_type_link == RESOURCE_TYPE_LINK_SERIES and
49+ task.target.project in all_projects):
50+ continue # Skip series task status for our projects
51+ if (task.assignee is not None and
52+ task.assignee.name not in all_users):
53+ continue # Skip non-users tasks
54 if (task.target in all_projects or
55- (task.assignee is not None and
56- task.assignee.name in all_users)):
57+ task.assignee is not None):
58 matches = True
59 if (lowest_status is None or
60 (ORDERED_BUG_STATUSES.index(task.status) <
61
62=== modified file 'src/lp2kanban/kanban.py'
63--- src/lp2kanban/kanban.py 2016-03-15 11:37:52 +0000
64+++ src/lp2kanban/kanban.py 2016-04-15 17:27:22 +0000
65@@ -16,6 +16,8 @@
66 r'(?P<branch>~(?P<owner>[-.\w]*)(/[-.\w]+){2,4})'
67 '(/\+merge/\d+)?/?$')
68
69+RESOURCE_TYPE_LINK_SERIES = "https://api.launchpad.net/devel/#project_series"
70+
71
72 class Record(dict):
73 """A little dict subclass that adds attribute access to values."""
74
75=== modified file 'src/lp2kanban/tests/test_bugs2cards.py'
76--- src/lp2kanban/tests/test_bugs2cards.py 2016-04-13 16:06:23 +0000
77+++ src/lp2kanban/tests/test_bugs2cards.py 2016-04-15 17:27:22 +0000
78@@ -33,6 +33,7 @@
79 sync_board
80 )
81 from lp2kanban.kanban import (
82+ RESOURCE_TYPE_LINK_SERIES,
83 LeankitCard,
84 Record,
85 )
86@@ -290,6 +291,13 @@
87 self.saved = True
88
89
90+class FauxTarget:
91+
92+ def __init__(self, resource_type_link="http://api/project", project=None):
93+ self.resource_type_link = resource_type_link
94+ self.project = project
95+
96+
97 class FauxBugTask:
98
99 def __init__(self, bug_id, target='faux-project', tags=None,
100@@ -307,7 +315,8 @@
101 # For a bug on an unrelated project and not assigned to any
102 # of the relevant users, it is not considered to match at all.
103 bug = FauxBug()
104- bug.addTask('project')
105+ project = FauxTarget(resource_type_link="project", project="project1")
106+ bug.addTask(project)
107 matches, status, assignees, mp_url, mp_type = (
108 get_bug_status(bug, ['launchpad'], []))
109 self.assertFalse(matches)
110@@ -319,9 +328,12 @@
111 # we care about, it is returned as matching with proper status.
112 bug = FauxBug()
113 user = Record(name='user')
114- bug.addTask('project', status='In Progress', assignee=user)
115+ project = FauxTarget(resource_type_link="project", project="project1")
116+ other_project = FauxTarget(
117+ resource_type_link="project", project="launchpad")
118+ bug.addTask(project, status='In Progress', assignee=user)
119 matches, status, assignees, mp_url, mp_type = (
120- get_bug_status(bug, ['launchpad'], ['user']))
121+ get_bug_status(bug, [other_project], ['user']))
122 self.assertTrue(matches)
123 self.assertEqual(CardStatus.CODING, status)
124 self.assertEqual([user], assignees)
125@@ -331,14 +343,17 @@
126 # as matching with a status that reflects the project task ignoring
127 # non-project tasks.
128 bug = FauxBug()
129- nonuser = Record(name='nonuser')
130- bug.addTask('launchpad', status='In Progress', assignee=nonuser)
131- bug.addTask('launchpad/16.03', status='Fix Released', assignee=nonuser)
132+ user = Record(name='user')
133+ project = FauxTarget(resource_type_link="project", project="project")
134+ series = FauxTarget(
135+ resource_type_link=RESOURCE_TYPE_LINK_SERIES, project=project)
136+ bug.addTask(project, status='In Progress', assignee=user)
137+ bug.addTask(series, status='Fix Released', assignee=user)
138 matches, status, assignees, mp_url, mp_type = (
139- get_bug_status(bug, ['launchpad'], []))
140+ get_bug_status(bug, [project], ['user']))
141 self.assertTrue(matches)
142 self.assertEqual(CardStatus.CODING, status)
143- self.assertEqual([nonuser], assignees)
144+ self.assertEqual([user], assignees)
145
146 def test_get_bug_status_multi_targets(self):
147 # For a bug with multiple tasks, only some of which are on
148@@ -347,10 +362,13 @@
149 bug = FauxBug()
150 nonuser = Record(name='nonuser')
151 user = Record(name='user')
152- bug.addTask('project', status='In Progress', assignee=nonuser)
153- bug.addTask('launchpad', status='Fix Released', assignee=user)
154+ project = FauxTarget(resource_type_link="project", project="project")
155+ other_project = FauxTarget(
156+ resource_type_link="project", project="launchpad")
157+ bug.addTask(project, status='In Progress', assignee=nonuser)
158+ bug.addTask(other_project, status='Fix Released', assignee=user)
159 matches, status, assignees, mp_url, mp_type = (
160- get_bug_status(bug, ['launchpad'], []))
161+ get_bug_status(bug, [project], ["user"]))
162 self.assertTrue(matches)
163 self.assertEqual(CardStatus.DONE_FIX, status)
164 self.assertEqual([user], assignees)
165@@ -361,10 +379,12 @@
166 # "lowest" of all bug task statuses (i.e. 'In Progress' is lower
167 # than 'Fix Committed').
168 bug = FauxBug()
169- bug.addTask('lazr', status='Fix Released')
170- bug.addTask('launchpad', status='In Progress')
171+ project1 = FauxTarget(resource_type_link="project", project="lazr")
172+ project2 = FauxTarget(resource_type_link="project", project="launchpad")
173+ bug.addTask(project1, status='Fix Released')
174+ bug.addTask(project2, status='In Progress')
175 matches, status, assignees, mp_url, mp_type = get_bug_status(
176- bug, ['launchpad', 'lazr'], [])
177+ bug, [project1, project2], [])
178 self.assertTrue(matches)
179 # 'Fix Released' on 'lazr' would put the bug status in DONE state,
180 # but since there is an 'In Progress' task as well, it is instead
181@@ -611,18 +631,19 @@
182 def test_get_card_status_new_no_branch(self):
183 # For a bug in 'New', 'Triaged' or 'Confirmed' or 'Incomplete' statuses,
184 # the branch status is new state.
185- self.assertEqual(
186- CardStatus.NEW,
187- get_card_status('New', [], None))
188- self.assertEqual(
189- CardStatus.NEW,
190- get_card_status('Confirmed', [], None))
191- self.assertEqual(
192- CardStatus.NEW,
193- get_card_status('Triaged', [], None))
194- self.assertEqual(
195- CardStatus.NEW,
196- get_card_status('Incomplete', [], None))
197+ no_branch_info = Record(status=None)
198+ self.assertEqual(
199+ CardStatus.NEW,
200+ get_card_status('New', [], no_branch_info))
201+ self.assertEqual(
202+ CardStatus.NEW,
203+ get_card_status('Confirmed', [], no_branch_info))
204+ self.assertEqual(
205+ CardStatus.NEW,
206+ get_card_status('Triaged', [], no_branch_info))
207+ self.assertEqual(
208+ CardStatus.NEW,
209+ get_card_status('Incomplete', [], no_branch_info))
210
211 def test_get_card_status_coding_no_bug(self):
212 # For a card with no associated bug, an 'In Progress' branch status
213@@ -651,15 +672,21 @@
214 CardStatus.LANDING,
215 get_card_status(None, [], branch_info))
216
217- def test_get_card_status_coding_no_branch(self):
218- # For a bug in 'In Progress' or 'Fix Committed' status, it is
219- # considered to be in the coding state if there is no branch.
220+ def test_get_card_status_coding_in_progress_no_branch(self):
221+ # For a bug in 'In Progress' status, it is considered to be in the
222+ # coding state if there is no branch.
223 branch_info = Record(status=None, target=None)
224 self.assertEqual(
225 CardStatus.CODING,
226 get_card_status('In Progress', [], branch_info))
227+
228+ def test_get_card_status_deploy_fix_committed_no_branch(self):
229+ # For a bug in 'Fix Committed' status, it is considered to be in the
230+ # deploy phase when there is no branch linked as the bug is considered
231+ # fixed with a trivial.
232+ branch_info = Record(status=None, target=None)
233 self.assertEqual(
234- CardStatus.CODING,
235+ CardStatus.DEPLOY,
236 get_card_status('Fix Committed', [], branch_info))
237
238 def test_get_card_status_coding_branch(self):
239@@ -741,19 +768,25 @@
240 def test_get_card_status_done_with_fix(self):
241 # For a bug in 'Fix Released' status, it is considered completely done.
242 self.assertEqual(
243- CardStatus.DONE_FIX, get_card_status('Fix Released', [], None))
244+ CardStatus.DONE_FIX,
245+ get_card_status('Fix Released', [], Record(status=None)))
246
247 def test_get_card_status_done_with_no_fix(self):
248 # For a bug in 'Opinion', 'Won't fix', 'Invalid' or'Expired' statuses,
249 # it is considered completely done without a fix
250- self.assertEqual(
251- CardStatus.DONE_NOFIX, get_card_status('Opinion', [], None))
252- self.assertEqual(
253- CardStatus.DONE_NOFIX, get_card_status("Won't Fix", [], None))
254- self.assertEqual(
255- CardStatus.DONE_NOFIX, get_card_status('Invalid', [], None))
256- self.assertEqual(
257- CardStatus.DONE_NOFIX, get_card_status('Expired', [], None))
258+ no_branch_info = Record(status=None)
259+ self.assertEqual(
260+ CardStatus.DONE_NOFIX,
261+ get_card_status('Opinion', [], no_branch_info))
262+ self.assertEqual(
263+ CardStatus.DONE_NOFIX,
264+ get_card_status("Won't Fix", [], no_branch_info))
265+ self.assertEqual(
266+ CardStatus.DONE_NOFIX,
267+ get_card_status('Invalid', [], no_branch_info))
268+ self.assertEqual(
269+ CardStatus.DONE_NOFIX,
270+ get_card_status('Expired', [], no_branch_info))
271
272
273 class GetCardsForFeatureTest(unittest.TestCase):

Subscribers

People subscribed via source and target branches

to all changes: