Skip to content

Commit 9f2ad7e

Browse files
Copilotzkoppert
andauthored
fix: label metrics overcounting for closed issues when labels removed before closure (#571)
* Initial plan * Fix label metrics overcounting bug and add comprehensive tests Co-authored-by: zkoppert <[email protected]> * Add 3 additional test cases for label metrics with creation-time labels and varied timeframes Co-authored-by: zkoppert <[email protected]> * Fix black linting issue with long function name formatting Co-authored-by: zkoppert <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: zkoppert <[email protected]> Co-authored-by: Zack Koppert <[email protected]>
1 parent 2212538 commit 9f2ad7e

File tree

2 files changed

+179
-1
lines changed

2 files changed

+179
-1
lines changed

labels.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,14 @@ def get_label_metrics(issue: github3.issues.Issue, labels: List[str]) -> dict:
8585
if label in labeled:
8686
# if the issue is closed, add the time from the issue creation to the closed_at time
8787
if issue.state == "closed":
88+
# Only add the final (closed_at - created_at) span if the label was still applied at closure.
89+
if label_last_event_type.get(label) != "labeled":
90+
continue
8891
label_metrics[label] += datetime.fromisoformat(
8992
issue.closed_at
9093
) - datetime.fromisoformat(issue.created_at)
9194
else:
92-
# skip label if last labeling event is 'unlabled' and issue is still open
95+
# skip label if last labeling event is 'unlabeled' and issue is still open
9396
if label_last_event_type[label] == "unlabeled":
9497
continue
9598

test_labels.py

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,181 @@ def test_get_label_metrics_closed_issue_labeled_past_closed_at(self):
9393
metrics = get_label_metrics(self.issue, labels)
9494
self.assertEqual(metrics["foo"], None)
9595

96+
def test_get_label_metrics_closed_issue_label_removed_before_closure(self):
97+
"""Test get_label_metrics for a closed issue where label was removed before closure"""
98+
# Create a mock issue that reproduces the problem scenario:
99+
# Issue created: day 0 (2021-01-01)
100+
# Label added: day 5 (2021-01-06)
101+
# Label removed: day 10 (2021-01-11)
102+
# Issue closed: day 15 (2021-01-16)
103+
# Expected duration: 5 days (from day 5 to day 10)
104+
105+
issue = MagicMock()
106+
issue.issue = MagicMock(spec=github3.issues.Issue)
107+
issue.created_at = "2021-01-01T00:00:00Z"
108+
issue.closed_at = "2021-01-16T00:00:00Z" # 15 days after creation
109+
issue.state = "closed"
110+
issue.issue.events.return_value = [
111+
MagicMock(
112+
event="labeled",
113+
label={"name": "test-label"},
114+
created_at=datetime(2021, 1, 6, tzinfo=pytz.UTC), # day 5
115+
),
116+
MagicMock(
117+
event="unlabeled",
118+
label={"name": "test-label"},
119+
created_at=datetime(2021, 1, 11, tzinfo=pytz.UTC), # day 10
120+
),
121+
]
122+
123+
labels = ["test-label"]
124+
metrics = get_label_metrics(issue, labels)
125+
126+
# Should be 5 days (from day 5 to day 10), not 15 days (full issue duration)
127+
expected_duration = timedelta(days=5)
128+
self.assertEqual(metrics["test-label"], expected_duration)
129+
130+
def test_get_label_metrics_closed_issue_label_remains_through_closure(self):
131+
"""Test get_label_metrics for a closed issue where label remains applied through closure"""
132+
# Test scenario where label is applied and never removed:
133+
# Issue created: day 0 (2021-01-01)
134+
# Label added: day 2 (2021-01-03)
135+
# Issue closed: day 10 (2021-01-11)
136+
# Expected duration: 10 days (from issue creation to closure)
137+
138+
issue = MagicMock()
139+
issue.issue = MagicMock(spec=github3.issues.Issue)
140+
issue.created_at = "2021-01-01T00:00:00Z"
141+
issue.closed_at = "2021-01-11T00:00:00Z" # 10 days after creation
142+
issue.state = "closed"
143+
issue.issue.events.return_value = [
144+
MagicMock(
145+
event="labeled",
146+
label={"name": "stays-applied"},
147+
created_at=datetime(2021, 1, 3, tzinfo=pytz.UTC), # day 2
148+
),
149+
# No unlabel event - label remains applied
150+
]
151+
152+
labels = ["stays-applied"]
153+
metrics = get_label_metrics(issue, labels)
154+
155+
# Should be 8 days (from day 2 when label was applied to day 10 when issue closed)
156+
expected_duration = timedelta(days=8)
157+
self.assertEqual(metrics["stays-applied"], expected_duration)
158+
159+
def test_get_label_metrics_label_applied_at_creation_and_removed_before_closure(
160+
self,
161+
):
162+
"""Test get_label_metrics for a label applied at issue creation and removed before closure"""
163+
# Test scenario where label is applied at creation and later removed:
164+
# Issue created: day 0 (2021-01-01) with label applied
165+
# Label removed: day 7 (2021-01-08)
166+
# Issue closed: day 20 (2021-01-21)
167+
# Expected duration: 7 days (from creation to removal)
168+
169+
issue = MagicMock()
170+
issue.issue = MagicMock(spec=github3.issues.Issue)
171+
issue.created_at = "2021-01-01T00:00:00Z"
172+
issue.closed_at = "2021-01-21T00:00:00Z" # 20 days after creation
173+
issue.state = "closed"
174+
issue.issue.events.return_value = [
175+
MagicMock(
176+
event="labeled",
177+
label={"name": "creation-label"},
178+
created_at=datetime(2021, 1, 1, tzinfo=pytz.UTC), # day 0 - at creation
179+
),
180+
MagicMock(
181+
event="unlabeled",
182+
label={"name": "creation-label"},
183+
created_at=datetime(2021, 1, 8, tzinfo=pytz.UTC), # day 7
184+
),
185+
]
186+
187+
labels = ["creation-label"]
188+
metrics = get_label_metrics(issue, labels)
189+
190+
# Should be 7 days (from creation to removal), not 20 days (full issue duration)
191+
expected_duration = timedelta(days=7)
192+
self.assertEqual(metrics["creation-label"], expected_duration)
193+
194+
def test_get_label_metrics_label_applied_at_creation_remains_through_closure(self):
195+
"""Test get_label_metrics for a label applied at creation and kept through closure"""
196+
# Test scenario where label is applied at creation and never removed:
197+
# Issue created: day 0 (2021-01-01) with label applied
198+
# Issue closed: day 30 (2021-01-31)
199+
# Expected duration: 30 days (full issue duration)
200+
201+
issue = MagicMock()
202+
issue.issue = MagicMock(spec=github3.issues.Issue)
203+
issue.created_at = "2021-01-01T00:00:00Z"
204+
issue.closed_at = "2021-01-31T00:00:00Z" # 30 days after creation
205+
issue.state = "closed"
206+
issue.issue.events.return_value = [
207+
MagicMock(
208+
event="labeled",
209+
label={"name": "permanent-label"},
210+
created_at=datetime(2021, 1, 1, tzinfo=pytz.UTC), # day 0 - at creation
211+
),
212+
# No unlabel event - label remains applied
213+
]
214+
215+
labels = ["permanent-label"]
216+
metrics = get_label_metrics(issue, labels)
217+
218+
# Should be 30 days (full issue duration since label was applied at creation)
219+
expected_duration = timedelta(days=30)
220+
self.assertEqual(metrics["permanent-label"], expected_duration)
221+
222+
def test_get_label_metrics_multiple_labels_different_timeframes(self):
223+
"""Test get_label_metrics with multiple labels having different application patterns and longer timeframes"""
224+
# Test scenario with multiple labels and longer timeframes:
225+
# Issue created: day 0 (2021-01-01)
226+
# Label A applied: day 0 (at creation)
227+
# Label B applied: day 14 (2021-01-15)
228+
# Label A removed: day 21 (2021-01-22)
229+
# Label B removed: day 35 (2021-02-05)
230+
# Issue closed: day 60 (2021-03-02)
231+
# Expected: Label A = 21 days, Label B = 21 days
232+
233+
issue = MagicMock()
234+
issue.issue = MagicMock(spec=github3.issues.Issue)
235+
issue.created_at = "2021-01-01T00:00:00Z"
236+
issue.closed_at = "2021-03-02T00:00:00Z" # 60 days after creation
237+
issue.state = "closed"
238+
issue.issue.events.return_value = [
239+
MagicMock(
240+
event="labeled",
241+
label={"name": "label-a"},
242+
created_at=datetime(2021, 1, 1, tzinfo=pytz.UTC), # day 0 - at creation
243+
),
244+
MagicMock(
245+
event="labeled",
246+
label={"name": "label-b"},
247+
created_at=datetime(2021, 1, 15, tzinfo=pytz.UTC), # day 14
248+
),
249+
MagicMock(
250+
event="unlabeled",
251+
label={"name": "label-a"},
252+
created_at=datetime(2021, 1, 22, tzinfo=pytz.UTC), # day 21
253+
),
254+
MagicMock(
255+
event="unlabeled",
256+
label={"name": "label-b"},
257+
created_at=datetime(2021, 2, 5, tzinfo=pytz.UTC), # day 35
258+
),
259+
]
260+
261+
labels = ["label-a", "label-b"]
262+
metrics = get_label_metrics(issue, labels)
263+
264+
# Label A: 21 days (from day 0 to day 21)
265+
# Label B: 21 days (from day 14 to day 35)
266+
expected_duration_a = timedelta(days=21)
267+
expected_duration_b = timedelta(days=21)
268+
self.assertEqual(metrics["label-a"], expected_duration_a)
269+
self.assertEqual(metrics["label-b"], expected_duration_b)
270+
96271

97272
class TestGetAverageTimeInLabels(unittest.TestCase):
98273
"""Unit tests for get_stats_time_in_labels"""

0 commit comments

Comments
 (0)