From 46336657717a610434ebe8b23ce034f18b86c8c3 Mon Sep 17 00:00:00 2001 From: Bregwin Jogi Date: Tue, 26 Nov 2024 17:50:46 -0500 Subject: [PATCH 1/3] Update time in draft to account for edge cases and update tests --- issue_metrics.py | 3 +-- test_time_in_draft.py | 62 ++++++++++++++++++++++++++++++++++++------- time_in_draft.py | 31 ++++++++++++++-------- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/issue_metrics.py b/issue_metrics.py index cdbaef9..0fd3a19 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -125,8 +125,7 @@ def get_per_issue_metrics( ready_for_review_at = get_time_to_ready_for_review(issue, pull_request) if env_vars.draft_pr_tracking: issue_with_metrics.time_in_draft = measure_time_in_draft( - issue=issue, - ready_for_review_at=ready_for_review_at, + issue=issue ) if env_vars.hide_time_to_first_response is False: diff --git a/test_time_in_draft.py b/test_time_in_draft.py index 486cb9b..ceea0b8 100644 --- a/test_time_in_draft.py +++ b/test_time_in_draft.py @@ -18,15 +18,17 @@ def setUp(self): Setup common test data and mocks. """ self.issue = MagicMock() - self.issue.issue.created_at = datetime(2021, 1, 1, tzinfo=pytz.utc) self.issue.issue.state = "open" def test_time_in_draft_with_ready_for_review(self): """ - Test measure_time_in_draft when ready_for_review_at is provided. + Test measure_time_in_draft with one draft and review interval. """ - ready_for_review_at = datetime(2021, 1, 3, tzinfo=pytz.utc) - result = measure_time_in_draft(self.issue, ready_for_review_at) + self.issue.events.return_value = [ + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 3, tzinfo=pytz.utc)), + ] + result = measure_time_in_draft(self.issue) expected = timedelta(days=2) self.assertEqual(result, expected, "The time in draft should be 2 days.") @@ -34,22 +36,62 @@ def test_time_in_draft_without_ready_for_review(self): """ Test measure_time_in_draft when ready_for_review_at is not provided and issue is still open. """ + self.issue.events.return_value = [ + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + ] now = datetime(2021, 1, 4, tzinfo=pytz.utc) with unittest.mock.patch("time_in_draft.datetime") as mock_datetime: mock_datetime.now.return_value = now - result = measure_time_in_draft(self.issue, None) + result = measure_time_in_draft(self.issue) expected = timedelta(days=3) self.assertEqual(result, expected, "The time in draft should be 3 days.") + + def test_time_in_draft_multiple_intervals(self): + """ + Test measure_time_in_draft with multiple draft intervals. + """ + self.issue.events.return_value = [ + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 3, tzinfo=pytz.utc)), + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 5, tzinfo=pytz.utc)), + MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 7, tzinfo=pytz.utc)), + ] + result = measure_time_in_draft(self.issue) + expected = timedelta(days=4) + self.assertEqual(result, expected, "The total time in draft should be 4 days.") + + def test_time_in_draft_ongoing_draft(self): + """ + Test measure_time_in_draft with an ongoing draft interval. + """ + self.issue.events.return_value = [ + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + ] + with unittest.mock.patch("time_in_draft.datetime") as mock_datetime: + mock_datetime.now.return_value = datetime(2021, 1, 4, tzinfo=pytz.utc) + result = measure_time_in_draft(self.issue) + expected = timedelta(days=3) + self.assertEqual(result, expected, "The ongoing draft time should be 3 days.") + + def test_time_in_draft_no_draft_events(self): + """ + Test measure_time_in_draft with no draft-related events. + """ + self.issue.events.return_value = [] + result = measure_time_in_draft(self.issue) + self.assertIsNone(result, "The result should be None when there are no draft events.") + def test_time_in_draft_without_ready_for_review_and_closed(self): """ - Test measure_time_in_draft when ready_for_review_at is not provided and issue is closed. + Test measure_time_in_draft for a closed issue with an ongoing draft and ready_for_review_at is not provided. """ + self.issue.events.return_value = [ + MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + ] self.issue.issue.state = "closed" - result = measure_time_in_draft(self.issue, None) - self.assertIsNone( - result, "The result should be None when draft was never used." - ) + result = measure_time_in_draft(self.issue) + self.assertIsNone(result, "The result should be None for a closed issue with an ongoing draft.") class TestGetStatsTimeInDraft(unittest.TestCase): diff --git a/time_in_draft.py b/time_in_draft.py index 59fe24a..9244145 100644 --- a/time_in_draft.py +++ b/time_in_draft.py @@ -13,23 +13,32 @@ def measure_time_in_draft( issue: github3.issues.Issue, - ready_for_review_at: Union[datetime, None], -) -> Union[datetime, None]: - """If a pull request has had time in the draft state, return the amount of time it was in draft. +) -> Union[timedelta, None]: + """If a pull request has had time in the draft state, return the cumulative amount of time it was in draft. args: issue (github3.issues.Issue): A GitHub issue which has been pre-qualified as a pull request. - ready_for_review_at (datetime | None): The time the pull request was marked as - ready for review. returns: - Union[datetime, None]: The time the pull request was in draft state. + Union[timedelta, None]: Total time the pull request has spent in draft state. """ - if ready_for_review_at: - return ready_for_review_at - issue.issue.created_at - if issue.issue.state == "open": - return datetime.now(pytz.utc) - issue.issue.created_at - return None + events = issue.events() + draft_start = None + total_draft_time = timedelta(0) + + for event in events: + if event.event == "converted_to_draft": + draft_start = event.created_at + elif event.event == "ready_for_review" and draft_start: + # Calculate draft time for this interval + total_draft_time += event.created_at - draft_start + draft_start = None + + # If the PR is currently in draft state, calculate the time in draft up to now + if draft_start and issue.issue.state == "open": + total_draft_time += datetime.now(pytz.utc) - draft_start + + return total_draft_time if total_draft_time > timedelta(0) else None def get_stats_time_in_draft( From de48501de5e541e565dde97d86479c3870b4b4ff Mon Sep 17 00:00:00 2001 From: Bregwin Jogi Date: Tue, 26 Nov 2024 18:07:44 -0500 Subject: [PATCH 2/3] Fix lint issues --- test_time_in_draft.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test_time_in_draft.py b/test_time_in_draft.py index ceea0b8..1cfa067 100644 --- a/test_time_in_draft.py +++ b/test_time_in_draft.py @@ -46,7 +46,6 @@ def test_time_in_draft_without_ready_for_review(self): expected = timedelta(days=3) self.assertEqual(result, expected, "The time in draft should be 3 days.") - def test_time_in_draft_multiple_intervals(self): """ Test measure_time_in_draft with multiple draft intervals. From 7429d773172d1311cb96c94d5c802dae655ab847 Mon Sep 17 00:00:00 2001 From: Zack Koppert Date: Sun, 1 Dec 2024 09:12:39 -0800 Subject: [PATCH 3/3] chore: fix style by running black lint formatter Signed-off-by: Zack Koppert --- test_time_in_draft.py | 58 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/test_time_in_draft.py b/test_time_in_draft.py index 1cfa067..2475147 100644 --- a/test_time_in_draft.py +++ b/test_time_in_draft.py @@ -25,8 +25,14 @@ def test_time_in_draft_with_ready_for_review(self): Test measure_time_in_draft with one draft and review interval. """ self.issue.events.return_value = [ - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), - MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 3, tzinfo=pytz.utc)), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 1, tzinfo=pytz.utc), + ), + MagicMock( + event="ready_for_review", + created_at=datetime(2021, 1, 3, tzinfo=pytz.utc), + ), ] result = measure_time_in_draft(self.issue) expected = timedelta(days=2) @@ -37,7 +43,10 @@ def test_time_in_draft_without_ready_for_review(self): Test measure_time_in_draft when ready_for_review_at is not provided and issue is still open. """ self.issue.events.return_value = [ - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 1, tzinfo=pytz.utc), + ), ] now = datetime(2021, 1, 4, tzinfo=pytz.utc) with unittest.mock.patch("time_in_draft.datetime") as mock_datetime: @@ -51,10 +60,22 @@ def test_time_in_draft_multiple_intervals(self): Test measure_time_in_draft with multiple draft intervals. """ self.issue.events.return_value = [ - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), - MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 3, tzinfo=pytz.utc)), - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 5, tzinfo=pytz.utc)), - MagicMock(event="ready_for_review", created_at=datetime(2021, 1, 7, tzinfo=pytz.utc)), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 1, tzinfo=pytz.utc), + ), + MagicMock( + event="ready_for_review", + created_at=datetime(2021, 1, 3, tzinfo=pytz.utc), + ), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 5, tzinfo=pytz.utc), + ), + MagicMock( + event="ready_for_review", + created_at=datetime(2021, 1, 7, tzinfo=pytz.utc), + ), ] result = measure_time_in_draft(self.issue) expected = timedelta(days=4) @@ -65,13 +86,18 @@ def test_time_in_draft_ongoing_draft(self): Test measure_time_in_draft with an ongoing draft interval. """ self.issue.events.return_value = [ - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 1, tzinfo=pytz.utc), + ), ] with unittest.mock.patch("time_in_draft.datetime") as mock_datetime: mock_datetime.now.return_value = datetime(2021, 1, 4, tzinfo=pytz.utc) result = measure_time_in_draft(self.issue) expected = timedelta(days=3) - self.assertEqual(result, expected, "The ongoing draft time should be 3 days.") + self.assertEqual( + result, expected, "The ongoing draft time should be 3 days." + ) def test_time_in_draft_no_draft_events(self): """ @@ -79,18 +105,26 @@ def test_time_in_draft_no_draft_events(self): """ self.issue.events.return_value = [] result = measure_time_in_draft(self.issue) - self.assertIsNone(result, "The result should be None when there are no draft events.") + self.assertIsNone( + result, "The result should be None when there are no draft events." + ) def test_time_in_draft_without_ready_for_review_and_closed(self): """ Test measure_time_in_draft for a closed issue with an ongoing draft and ready_for_review_at is not provided. """ self.issue.events.return_value = [ - MagicMock(event="converted_to_draft", created_at=datetime(2021, 1, 1, tzinfo=pytz.utc)), + MagicMock( + event="converted_to_draft", + created_at=datetime(2021, 1, 1, tzinfo=pytz.utc), + ), ] self.issue.issue.state = "closed" result = measure_time_in_draft(self.issue) - self.assertIsNone(result, "The result should be None for a closed issue with an ongoing draft.") + self.assertIsNone( + result, + "The result should be None for a closed issue with an ongoing draft.", + ) class TestGetStatsTimeInDraft(unittest.TestCase):