Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Ignore PR time spent as draft, measure PRs by merge time #111

Merged
merged 2 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
16 changes: 14 additions & 2 deletions issue_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
67 changes: 50 additions & 17 deletions test_time_to_first_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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

Expand All @@ -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()
Expand All @@ -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
Expand Down
51 changes: 51 additions & 0 deletions test_time_to_merge.py
Original file line number Diff line number Diff line change
@@ -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))
57 changes: 57 additions & 0 deletions test_time_to_ready_for_review.py
Original file line number Diff line number Diff line change
@@ -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)
18 changes: 14 additions & 4 deletions time_to_first_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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:
Expand All @@ -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

Expand All @@ -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(
Expand Down
Loading