Skip to content

Commit 5e6d889

Browse files
zkoppertzkoppert
committed
fix: issue counts incorrect when HIDE_TIME_TO_CLOSE is True
Signed-off-by: Zack Koppert <[email protected]>
1 parent cd573ba commit 5e6d889

File tree

3 files changed

+192
-27
lines changed

3 files changed

+192
-27
lines changed

‎issue_metrics.py

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,12 @@ def get_per_issue_metrics(
193193
)
194194
if env_vars.hide_time_to_answer is False:
195195
issue_with_metrics.time_to_answer = measure_time_to_answer(issue)
196-
if env_vars.hide_time_to_close is False:
197-
if issue["closedAt"]:
198-
issue_with_metrics.time_to_close = measure_time_to_close(
199-
None, issue
200-
)
201-
num_issues_closed += 1
202-
else:
203-
num_issues_open += 1
196+
if env_vars.hide_time_to_close is False and issue["closedAt"]:
197+
issue_with_metrics.time_to_close = measure_time_to_close(None, issue)
198+
if issue["closedAt"]:
199+
num_issues_closed += 1
200+
else:
201+
num_issues_open += 1
204202
else:
205203
issue_with_metrics = IssueWithMetrics(
206204
issue.title, # type: ignore
@@ -236,19 +234,19 @@ def get_per_issue_metrics(
236234
)
237235
if labels and env_vars.hide_label_metrics is False:
238236
issue_with_metrics.label_metrics = get_label_metrics(issue, labels)
239-
if env_vars.hide_time_to_close is False:
240-
if issue.state == "closed": # type: ignore
241-
if pull_request:
242-
issue_with_metrics.time_to_close = measure_time_to_merge(
243-
pull_request, ready_for_review_at
244-
)
245-
else:
246-
issue_with_metrics.time_to_close = measure_time_to_close(
247-
issue, None
248-
)
249-
num_issues_closed += 1
250-
elif issue.state == "open": # type: ignore
251-
num_issues_open += 1
237+
if env_vars.hide_time_to_close is False and issue.state == "closed": # type: ignore
238+
if pull_request:
239+
issue_with_metrics.time_to_close = measure_time_to_merge(
240+
pull_request, ready_for_review_at
241+
)
242+
else:
243+
issue_with_metrics.time_to_close = measure_time_to_close(
244+
issue, None
245+
)
246+
if issue.state == "closed": # type: ignore
247+
num_issues_closed += 1
248+
elif issue.state == "open": # type: ignore
249+
num_issues_open += 1
252250
issues_with_metrics.append(issue_with_metrics)
253251

254252
return issues_with_metrics, num_issues_open, num_issues_closed

‎test_issue_metrics.py

Lines changed: 171 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,131 @@ class TestGetPerIssueMetrics(unittest.TestCase):
175175

176176
@patch.dict(
177177
os.environ,
178-
{"GH_TOKEN": "test_token", "SEARCH_QUERY": "is:issue is:open repo:user/repo"},
178+
{
179+
"GH_TOKEN": "test_token",
180+
"SEARCH_QUERY": "is:issue is:open repo:user/repo",
181+
"HIDE_AUTHOR": "true",
182+
"HIDE_LABEL_METRICS": "true",
183+
"HIDE_TIME_TO_ANSWER": "true",
184+
"HIDE_TIME_TO_CLOSE": "true",
185+
"HIDE_TIME_TO_FIRST_RESPONSE": "true",
186+
},
187+
)
188+
def test_get_per_issue_metrics_with_hide_envs(self):
189+
"""
190+
Test that the function correctly calculates the metrics for
191+
a list of GitHub issues where HIDE_* envs are set true
192+
"""
193+
194+
# Create mock data
195+
mock_issue1 = MagicMock(
196+
title="Issue 1",
197+
html_url="https://github.com/user/repo/issues/1",
198+
author="alice",
199+
state="open",
200+
comments=1,
201+
created_at="2023-01-01T00:00:00Z",
202+
)
203+
204+
mock_comment1 = MagicMock()
205+
mock_comment1.created_at = datetime.fromisoformat("2023-01-02T00:00:00Z")
206+
mock_issue1.issue.comments.return_value = [mock_comment1]
207+
mock_issue1.issue.pull_request_urls = None
208+
209+
mock_issue2 = MagicMock(
210+
title="Issue 2",
211+
html_url="https://github.com/user/repo/issues/2",
212+
author="bob",
213+
state="closed",
214+
comments=1,
215+
created_at="2023-01-01T00:00:00Z",
216+
closed_at="2023-01-04T00:00:00Z",
217+
)
218+
219+
mock_comment2 = MagicMock()
220+
mock_comment2.created_at = datetime.fromisoformat("2023-01-03T00:00:00Z")
221+
mock_issue2.issue.comments.return_value = [mock_comment2]
222+
mock_issue2.issue.pull_request_urls = None
223+
224+
issues = [
225+
mock_issue1,
226+
mock_issue2,
227+
]
228+
229+
# Call the function and check the result
230+
with unittest.mock.patch( # type:ignore
231+
"issue_metrics.measure_time_to_first_response",
232+
measure_time_to_first_response,
233+
), unittest.mock.patch( # type:ignore
234+
"issue_metrics.measure_time_to_close", measure_time_to_close
235+
):
236+
(
237+
result_issues_with_metrics,
238+
result_num_issues_open,
239+
result_num_issues_closed,
240+
) = get_per_issue_metrics(
241+
issues,
242+
env_vars=get_env_vars(test=True),
243+
)
244+
expected_issues_with_metrics = [
245+
IssueWithMetrics(
246+
"Issue 1",
247+
"https://github.com/user/repo/issues/1",
248+
"alice",
249+
None,
250+
None,
251+
None,
252+
None,
253+
),
254+
IssueWithMetrics(
255+
"Issue 2",
256+
"https://github.com/user/repo/issues/2",
257+
"bob",
258+
None,
259+
None,
260+
None,
261+
None,
262+
),
263+
]
264+
expected_num_issues_open = 1
265+
expected_num_issues_closed = 1
266+
self.assertEqual(result_num_issues_open, expected_num_issues_open)
267+
self.assertEqual(result_num_issues_closed, expected_num_issues_closed)
268+
self.assertEqual(
269+
result_issues_with_metrics[0].time_to_first_response,
270+
expected_issues_with_metrics[0].time_to_first_response,
271+
)
272+
self.assertEqual(
273+
result_issues_with_metrics[0].time_to_close,
274+
expected_issues_with_metrics[0].time_to_close,
275+
)
276+
self.assertEqual(
277+
result_issues_with_metrics[1].time_to_first_response,
278+
expected_issues_with_metrics[1].time_to_first_response,
279+
)
280+
self.assertEqual(
281+
result_issues_with_metrics[1].time_to_close,
282+
expected_issues_with_metrics[1].time_to_close,
283+
)
284+
285+
@patch.dict(
286+
os.environ,
287+
{
288+
"GH_TOKEN": "test_token",
289+
"SEARCH_QUERY": "is:issue is:open repo:user/repo",
290+
"HIDE_AUTHOR": "false",
291+
"HIDE_LABEL_METRICS": "false",
292+
"HIDE_TIME_TO_ANSWER": "false",
293+
"HIDE_TIME_TO_CLOSE": "false",
294+
"HIDE_TIME_TO_FIRST_RESPONSE": "false",
295+
},
179296
)
180-
def test_get_per_issue_metrics(self):
181-
"""Test that the function correctly calculates the metrics for a list of GitHub issues."""
297+
def test_get_per_issue_metrics_without_hide_envs(self):
298+
"""
299+
Test that the function correctly calculates the metrics for
300+
a list of GitHub issues where HIDE_* envs are set false
301+
"""
302+
182303
# Create mock data
183304
mock_issue1 = MagicMock(
184305
title="Issue 1",
@@ -225,7 +346,10 @@ def test_get_per_issue_metrics(self):
225346
result_issues_with_metrics,
226347
result_num_issues_open,
227348
result_num_issues_closed,
228-
) = get_per_issue_metrics(issues, env_vars=get_env_vars(test=True))
349+
) = get_per_issue_metrics(
350+
issues,
351+
env_vars=get_env_vars(test=True),
352+
)
229353
expected_issues_with_metrics = [
230354
IssueWithMetrics(
231355
"Issue 1",
@@ -337,6 +461,49 @@ def test_get_per_issue_metrics_with_discussion(self):
337461
self.assertEqual(metrics[0][1].time_to_close, timedelta(days=6))
338462
self.assertEqual(metrics[0][1].time_to_first_response, timedelta(days=2))
339463

464+
@patch.dict(
465+
os.environ,
466+
{
467+
"GH_TOKEN": "test_token",
468+
"SEARCH_QUERY": "is:issue is:open repo:user/repo",
469+
"HIDE_AUTHOR": "true",
470+
"HIDE_LABEL_METRICS": "true",
471+
"HIDE_TIME_TO_ANSWER": "true",
472+
"HIDE_TIME_TO_CLOSE": "true",
473+
"HIDE_TIME_TO_FIRST_RESPONSE": "true",
474+
},
475+
)
476+
def test_get_per_issue_metrics_with_discussion_with_hide_envs(self):
477+
"""
478+
Test that the function correctly calculates
479+
the metrics for a list of GitHub issues with discussions
480+
and HIDE_* env vars set to True
481+
"""
482+
483+
issues = [self.issue1, self.issue2]
484+
metrics = get_per_issue_metrics(
485+
issues, discussions=True, env_vars=get_env_vars(test=True)
486+
)
487+
488+
# get_per_issue_metrics returns a tuple of
489+
# (issues_with_metrics, num_issues_open, num_issues_closed)
490+
self.assertEqual(len(metrics), 3)
491+
492+
# Check that the metrics are correct, 0 issues open, 2 issues closed
493+
self.assertEqual(metrics[1], 0)
494+
self.assertEqual(metrics[2], 2)
495+
496+
# Check that the issues_with_metrics has 2 issues in it
497+
self.assertEqual(len(metrics[0]), 2)
498+
499+
# Check that the issues_with_metrics has the correct metrics,
500+
self.assertEqual(metrics[0][0].time_to_answer, None)
501+
self.assertEqual(metrics[0][0].time_to_close, None)
502+
self.assertEqual(metrics[0][0].time_to_first_response, None)
503+
self.assertEqual(metrics[0][1].time_to_answer, None)
504+
self.assertEqual(metrics[0][1].time_to_close, None)
505+
self.assertEqual(metrics[0][1].time_to_first_response, None)
506+
340507

341508
if __name__ == "__main__":
342509
unittest.main()

‎test_markdown_writer.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ def test_writes_markdown_file_with_non_hidden_columns_only(self):
297297
"label1": timedelta(days=1),
298298
}
299299
num_issues_opened = 2
300-
num_issues_closed = 1
300+
num_issues_closed = 2
301301
num_mentor_count = 5
302302

303303
# Call the function
@@ -323,7 +323,7 @@ def test_writes_markdown_file_with_non_hidden_columns_only(self):
323323
"| Metric | Count |\n"
324324
"| --- | ---: |\n"
325325
"| Number of items that remain open | 2 |\n"
326-
"| Number of items closed | 1 |\n"
326+
"| Number of items closed | 2 |\n"
327327
"| Number of most active mentors | 5 |\n"
328328
"| Total number of items created | 2 |\n\n"
329329
"| Title | URL | Author |\n"

0 commit comments

Comments
 (0)