diff --git a/README.md b/README.md index a0cbb07..a7a9fc9 100644 --- a/README.md +++ b/README.md @@ -8,11 +8,13 @@ several metrics. The issues/pull requests/discussions to search for can be filte The metrics that are measured are: | Metric | Description | |--------|-------------| -| Time to first response | The time between when an issue/pull request/discussion is created and when the first comment or review is made. | -| Time to close | The time between when an issue/pull request/discussion is created and when it is closed. | +| Time to first response | The time between when an issue/pull request/discussion is created and when the first comment or review is made.* | +| Time to close | The time between when an issue/pull request/discussion is created and when it is closed.* | | Time to answer | (Discussions only) The time between when a discussion is created and when it is answered. | | Time in label | The time between when a label has a specific label applied to an issue/pull request/discussion and when it is removed. This requires the LABELS_TO_MEASURE env variable to be set. | +*For pull requests, these metrics exclude the time the PR was in draft mode. + This action was developed by the GitHub OSPO for our own use and developed in a way that we could open source it that it might be useful to you as well! If you want to know more about how we use it, reach out in an issue in this repository. To find syntax for search queries, check out the [documentation on searching issues and pull requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/filtering-and-searching-issues-and-pull-requests) or the [documentation on searching discussions](https://docs.github.com/en/search-github/searching-on-github/searching-discussions). diff --git a/issue_metrics.py b/issue_metrics.py index 74fcef1..33d534e 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -36,6 +36,8 @@ from markdown_writer import write_to_markdown from time_to_answer import get_average_time_to_answer, measure_time_to_answer from time_to_close import get_average_time_to_close, measure_time_to_close +from time_to_ready_for_review import get_time_to_ready_for_review +from time_to_merge import measure_time_to_merge from time_to_first_response import ( get_average_time_to_first_response, measure_time_to_first_response, @@ -193,13 +195,23 @@ def get_per_issue_metrics( None, None, ) + + # Check if issue is actually a pull request + pull_request, ready_for_review_at = None, None + if issue.issue.pull_request_urls: + pull_request = issue.issue.pull_request() + ready_for_review_at = get_time_to_ready_for_review(issue, pull_request) + issue_with_metrics.time_to_first_response = measure_time_to_first_response( - issue, None, ignore_users + issue, None, pull_request, ready_for_review_at, ignore_users ) if labels: issue_with_metrics.label_metrics = get_label_metrics(issue, labels) if issue.state == "closed": # type: ignore - issue_with_metrics.time_to_close = measure_time_to_close(issue, None) + if pull_request: + issue_with_metrics.time_to_close = measure_time_to_merge(pull_request, ready_for_review_at) + else: + issue_with_metrics.time_to_close = measure_time_to_close(issue, None) num_issues_closed += 1 elif issue.state == "open": # type: ignore num_issues_open += 1 diff --git a/test_time_to_first_response.py b/test_time_to_first_response.py index 58259c6..af7177b 100644 --- a/test_time_to_first_response.py +++ b/test_time_to_first_response.py @@ -74,17 +74,17 @@ def test_measure_time_to_first_response_with_pull_request_comments(self): mock_issue1.comments = 2 mock_issue1.issue.user.login = "issue_owner" mock_issue1.created_at = "2023-01-01T00:00:00Z" - mock_issue1.pull_request_urls = {"url": "https://api.github.com/repos/owner/repo/pulls/1"} # Set up the mock GitHub pull request comments mock_pr_comment1 = MagicMock() mock_pr_comment1.submitted_at = datetime.fromisoformat("2023-01-02T00:00:00Z") # first response mock_pr_comment2 = MagicMock() mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-02T12:00:00Z") - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1, mock_pr_comment2] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2] # Call the function - result = measure_time_to_first_response(mock_issue1, None) + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None) expected_result = timedelta(days=1) # Check the results @@ -98,7 +98,6 @@ def test_measure_time_to_first_response_issue_comment_faster(self): mock_issue1.comments = 2 mock_issue1.issue.user.login = "issue_owner" mock_issue1.created_at = "2023-01-01T00:00:00Z" - mock_issue1.pull_request_urls = {"url": "https://api.github.com/repos/owner/repo/pulls/1"} # Set up the mock GitHub issue comment mock_comment1 = MagicMock() @@ -108,10 +107,11 @@ def test_measure_time_to_first_response_issue_comment_faster(self): # Set up the mock GitHub pull request comment mock_pr_comment1 = MagicMock() mock_pr_comment1.submitted_at = datetime.fromisoformat("2023-01-03T00:00:00Z") - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1] # Call the function - result = measure_time_to_first_response(mock_issue1, None) + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None) expected_result = timedelta(days=1) # Check the results @@ -125,7 +125,6 @@ def test_measure_time_to_first_response_pull_request_comment_faster(self): mock_issue1.comments = 2 mock_issue1.issue.user.login = "issue_owner" mock_issue1.created_at = "2023-01-01T00:00:00Z" - mock_issue1.pull_request_urls = {"url": "https://api.github.com/repos/owner/repo/pulls/1"} # Set up the mock GitHub pull issue comment mock_comment1 = MagicMock() @@ -135,10 +134,43 @@ def test_measure_time_to_first_response_pull_request_comment_faster(self): # Set up the mock GitHub pull request comment mock_pr_comment1 = MagicMock() mock_pr_comment1.submitted_at = datetime.fromisoformat("2023-01-02T00:00:00Z") # first response - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1] # Call the function - result = measure_time_to_first_response(mock_issue1, None) + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None) + expected_result = timedelta(days=1) + + # Check the results + self.assertEqual(result, expected_result) + + def test_measure_time_to_first_response_pull_request_comment_ignore_before_ready(self): + """Test that measure_time_to_first_response ignores comments from before the pull request was ready for review.""" + # Set up the mock GitHub issues + mock_issue1 = MagicMock() + mock_issue1.comments = 4 + mock_issue1.issue.user.login = "issue_owner" + mock_issue1.created_at = "2023-01-01T00:00:00Z" + + # Set up the mock GitHub issue comments (one ignored, one not ignored) + mock_comment1 = MagicMock() + mock_comment1.created_at = datetime.fromisoformat("2023-01-02T00:00:00Z") + mock_comment2 = MagicMock() + mock_comment2.created_at = datetime.fromisoformat("2023-01-05T00:00:00Z") + mock_issue1.issue.comments.return_value = [mock_comment1, mock_comment2] + + # Set up the mock GitHub pull request comments (one ignored, one not ignored) + mock_pr_comment1 = MagicMock() + mock_pr_comment1.submitted_at = datetime.fromisoformat("2023-01-02T12:00:00Z") + mock_pr_comment2 = MagicMock() + mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-04T00:00:00Z") # first response + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2] + + ready_for_review_at = datetime.fromisoformat("2023-01-03T00:00:00Z") + + # Call the function + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, ready_for_review_at) expected_result = timedelta(days=1) # Check the results @@ -168,10 +200,11 @@ def test_measure_time_to_first_response_ignore_users(self): mock_pr_comment2 = MagicMock() mock_pr_comment2.user.login = "not_ignored_user" mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-04T00:00:00Z") # first response - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1, mock_pr_comment2] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2] # Call the function - result = measure_time_to_first_response(mock_issue1, None, ["ignored_user"]) + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None, ["ignored_user"]) expected_result = timedelta(days=3) # Check the results @@ -184,7 +217,6 @@ def test_measure_time_to_first_response_only_ignored_users(self): mock_issue1.comments = 4 mock_issue1.issue.user.login = "issue_owner" mock_issue1.created_at = "2023-01-01T00:00:00Z" - mock_issue1.pull_request_urls = {"url": "https://api.github.com/repos/owner/repo/pulls/1"} # Set up the mock GitHub issue comments (all ignored) mock_comment1 = MagicMock() @@ -202,11 +234,12 @@ def test_measure_time_to_first_response_only_ignored_users(self): mock_pr_comment2 = MagicMock() mock_pr_comment2.user.login = "ignored_user2" mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-04T12:00:00Z") - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1, mock_pr_comment2] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2] # Call the function result = measure_time_to_first_response( - mock_issue1, None, ["ignored_user", "ignored_user2"] + mock_issue1, None, mock_pull_request, None, ["ignored_user", "ignored_user2"] ) expected_result = None @@ -220,7 +253,6 @@ def test_measure_time_to_first_response_ignore_issue_owners_comment(self): mock_issue1.comments = 4 mock_issue1.issue.user.login = "issue_owner" mock_issue1.created_at = "2023-01-01T00:00:00Z" - mock_issue1.pull_request_urls = {"url": "https://api.github.com/repos/owner/repo/pulls/1"} # Set up the mock GitHub issue comments mock_comment1 = MagicMock() @@ -238,10 +270,11 @@ def test_measure_time_to_first_response_ignore_issue_owners_comment(self): mock_pr_comment2 = MagicMock() mock_pr_comment2.user.login = "other_user" mock_pr_comment2.submitted_at = datetime.fromisoformat("2023-01-04T00:00:00Z") - mock_issue1.issue.pull_request().reviews.return_value = [mock_pr_comment1, mock_pr_comment2] + mock_pull_request = MagicMock() + mock_pull_request.reviews.return_value = [mock_pr_comment1, mock_pr_comment2] # Call the function - result = measure_time_to_first_response(mock_issue1, None) + result = measure_time_to_first_response(mock_issue1, None, mock_pull_request, None) expected_result = timedelta(days=3) # Check the results diff --git a/test_time_to_merge.py b/test_time_to_merge.py new file mode 100644 index 0000000..abe5e86 --- /dev/null +++ b/test_time_to_merge.py @@ -0,0 +1,51 @@ +"""A module containing unit tests for the time_to_merge module. + +This module contains unit tests for the measure_time_to_merge +function in the time_to_merge module. +The tests use mock GitHub pull request to test the function's behavior. + +Classes: + TestMeasureTimeToMerge: A class to test the measure_time_to_merge function. + +""" +from datetime import timedelta, datetime +import unittest +from unittest.mock import MagicMock + +from time_to_merge import measure_time_to_merge + +class TestMeasureTimeToMerge(unittest.TestCase): + """Test suite for the measure_time_to_merge function.""" + + def test_measure_time_to_merge_ready_for_review(self): + """Test that the function correctly measures the time to merge a pull request that was formerly a draft.""" + # Create a mock pull request object + pull_request = MagicMock() + pull_request.merged_at = datetime.fromisoformat("2021-01-03T00:00:00Z") + ready_for_review_at = datetime.fromisoformat("2021-01-01T00:00:00Z") + + # Call the function and check the result + result = measure_time_to_merge(pull_request, ready_for_review_at) + expected_result = timedelta(days=2) + self.assertEqual(result, expected_result) + + def test_measure_time_to_merge_created_at(self): + """Test that the function correctly measures the time to merge a pull request that was never a draft.""" + # Create a mock pull request object + pull_request = MagicMock() + pull_request.merged_at = datetime.fromisoformat("2021-01-03T00:00:00Z") + pull_request.created_at = datetime.fromisoformat("2021-01-01T00:00:00Z") + + # Call the function and check the result + result = measure_time_to_merge(pull_request, None) + expected_result = timedelta(days=2) + self.assertEqual(result, expected_result) + + def test_measure_time_to_merge_returns_none(self): + """Test that the function returns None if the pull request is not merged.""" + # Create a mock issue object + pull_request = MagicMock() + pull_request.merged_at = None + + # Call the function and check that it returns None + self.assertEqual(None, measure_time_to_merge(pull_request, None)) diff --git a/test_time_to_ready_for_review.py b/test_time_to_ready_for_review.py new file mode 100644 index 0000000..16108ea --- /dev/null +++ b/test_time_to_ready_for_review.py @@ -0,0 +1,57 @@ +"""A module containing unit tests for the time_to_ready_for_review module. + +This module contains unit tests for the get_time_to_ready_for_review +function in the time_to_ready_for_review module. +The tests use mock GitHub issues to test the functions' behavior. + +Classes: + TestGetTimeToReadyForReview: A class to test the get_time_to_ready_for_review function. + +""" +from datetime import datetime +import unittest +from unittest.mock import MagicMock + +from time_to_ready_for_review import get_time_to_ready_for_review + +class TestGetTimeToReadyForReview(unittest.TestCase): + """Test suite for the get_time_to_ready_for_review function.""" + + # def draft pr function + def test_time_to_ready_for_review_draft(self): + """Test that the function returns None when the pull request is a draft""" + pull_request = MagicMock() + pull_request.draft = True + issue = MagicMock() + + result = get_time_to_ready_for_review(issue, pull_request) + expected_result = None + self.assertEqual(result, expected_result) + + def test_get_time_to_ready_for_review_event(self): + """Test that the function correctly gets the time a pull request was marked as ready for review""" + pull_request = MagicMock() + pull_request.draft = False + event = MagicMock() + event.event = 'ready_for_review' + event.created_at = datetime.fromisoformat("2021-01-01T00:00:00Z") + issue = MagicMock() + issue.issue.events.return_value=[event] + + result = get_time_to_ready_for_review(issue, pull_request) + expected_result = event.created_at + self.assertEqual(result, expected_result) + + def test_get_time_to_ready_for_review_no_event(self): + """Test that the function returns None when the pull request is not a draft and no ready_for_review event is found""" + pull_request = MagicMock() + pull_request.draft = False + event = MagicMock() + event.event = 'foobar' + event.created_at = "2021-01-01T00:00:00Z" + issue = MagicMock() + issue.events.return_value=[event] + + result = get_time_to_ready_for_review(issue, pull_request) + expected_result = None + self.assertEqual(result, expected_result) diff --git a/time_to_first_response.py b/time_to_first_response.py index 6e4fef9..b59702d 100644 --- a/time_to_first_response.py +++ b/time_to_first_response.py @@ -8,6 +8,7 @@ measure_time_to_first_response( issue: Union[github3.issues.Issue, None], discussion: Union[dict, None] + pull_request: Union[github3.pulls.PullRequest, None], ) -> Union[timedelta, None]: Measure the time to first response for a single issue or a discussion. get_average_time_to_first_response( @@ -27,13 +28,16 @@ def measure_time_to_first_response( issue: Union[github3.issues.Issue, None], # type: ignore discussion: Union[dict, None], + pull_request: Union[github3.pulls.PullRequest, None] = None, + ready_for_review_at: Union[datetime, None] = None, ignore_users: List[str] = None, ) -> Union[timedelta, None]: - """Measure the time to first response for a single issue or a discussion. + """Measure the time to first response for a single issue, pull request, or a discussion. Args: issue (Union[github3.issues.Issue, None]): A GitHub issue. discussion (Union[dict, None]): A GitHub discussion. + pull_request (Union[github3.pulls.PullRequest, None]): A GitHub pull request. ignore_users (List[str]): A list of GitHub usernames to ignore. Returns: @@ -57,19 +61,22 @@ def measure_time_to_first_response( continue if comment.user.login == issue.issue.user.login: continue + if ready_for_review_at and comment.created_at < ready_for_review_at: + continue first_comment_time = comment.created_at break # Check if the issue is actually a pull request # so we may also get the first review comment time - if issue.issue.pull_request_urls: - pull_request = issue.issue.pull_request() + if pull_request: review_comments = pull_request.reviews(number=50) # type: ignore for review_comment in review_comments: if review_comment.user.login in ignore_users: continue if review_comment.user.login == issue.issue.user.login: continue + if ready_for_review_at and review_comment.submitted_at < ready_for_review_at: + continue first_review_comment_time = review_comment.submitted_at break @@ -84,7 +91,10 @@ def measure_time_to_first_response( return None # Get the created_at time for the issue so we can calculate the time to first response - issue_time = datetime.fromisoformat(issue.created_at) # type: ignore + if ready_for_review_at: + issue_time = ready_for_review_at + else: + issue_time = datetime.fromisoformat(issue.created_at) # type: ignore if discussion and len(discussion["comments"]["nodes"]) > 0: earliest_response = datetime.fromisoformat( diff --git a/time_to_merge.py b/time_to_merge.py new file mode 100644 index 0000000..dbf3d86 --- /dev/null +++ b/time_to_merge.py @@ -0,0 +1,43 @@ +"""A module for measuring the time it takes to merge a GitHub pull request. + +This module provides functions for measuring the time it takes to merge a GitHub pull +request, as well as calculating the average time to merge for a list of pull requests. + +Functions: + measure_time_to_merge( + pull_request: github3.pulls.PullRequest, + ready_for_review_at: Union[datetime, None] + ) -> Union[timedelta, None]: + Measure the time it takes to merge a pull request. + +""" +from datetime import datetime, timedelta +from typing import List, Union + +import github3 + +def measure_time_to_merge( + pull_request: github3.pulls.PullRequest, + ready_for_review_at: Union[datetime, None] +) -> Union[timedelta, None]: + """Measure the time it takes to merge a pull request. + + Args: + pull_request (github3.pulls.PullRequest): A GitHub pull request. + ready_for_review_at (Union[timedelta, None]): When the PR was marked as ready for review + + Returns: + Union[datetime.timedelta, None]: The time it takes to close the issue. + + """ + merged_at = None + if pull_request.merged_at is None: + return None + + merged_at = pull_request.merged_at + + if ready_for_review_at: + return merged_at - ready_for_review_at + + created_at = pull_request.created_at + return merged_at - created_at diff --git a/time_to_ready_for_review.py b/time_to_ready_for_review.py new file mode 100644 index 0000000..ea8aa26 --- /dev/null +++ b/time_to_ready_for_review.py @@ -0,0 +1,44 @@ +"""A module for getting the time a pull request was marked as ready for review +after being in draft mode. + +This module provides functions for getting the time a GitHub pull request was +marked as ready for review, if it was formerly in draft mode. + +Functions: + get_time_to_ready_for_review( + issue: github3.issues.Issue, + pull_request: github3.pulls.PullRequest + ) -> Union[datetime, None]: + If a pull request was formerly a draft, get the time it was marked as + ready for review. + +""" + +from datetime import datetime +from typing import Union + +import github3 + +def get_time_to_ready_for_review( + issue: github3.issues.Issue, + pull_request: github3.pulls.PullRequest +) -> Union[datetime, None]: + """If a pull request was formerly a draft, get the time it was marked as ready + for review + + Args: + issue (github3.issues.Issue): A GitHub issue. + pull_request (github3.pulls.PullRequest): A GitHub pull request. + + Returns: + Union[datetime, None]: The time the pull request was marked as ready for review + """ + if pull_request.draft: + return None + + events = issue.issue.events(number=50) + for event in events: + if event.event == 'ready_for_review': + return event.created_at + + return None