From 901b160435106e031d6ab885ed846b1c47257fbe Mon Sep 17 00:00:00 2001 From: jmeridth Date: Thu, 23 May 2024 15:59:19 -0500 Subject: [PATCH] fix: issue accessing repo when not provided in query Fixes #290 - [x] handle trying to access repository from dictionary when not present (use default of empty string) - [x] add test to ensure we cover this scenario - [x] handle issue where `time_to_close` variable was being access before possibly being declared Signed-off-by: jmeridth --- issue_metrics.py | 4 +++- test_issue_metrics.py | 33 +++++++++++++++++++++++++++++---- time_to_close.py | 1 + 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/issue_metrics.py b/issue_metrics.py index be8327a..e5024a4 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -95,7 +95,9 @@ def wait_for_api_refresh(iterator: github3.structs.SearchIterator): issues = [] repos_and_owners_string = "" for item in owners_and_repositories: - repos_and_owners_string += f"{item['owner']}/{item['repository']} " + repos_and_owners_string += ( + f"{item.get('owner', '')}/{item.get('repository', '')} " + ) # Print the issue titles try: diff --git a/test_issue_metrics.py b/test_issue_metrics.py index a0bb3e9..1cf6d1d 100644 --- a/test_issue_metrics.py +++ b/test_issue_metrics.py @@ -36,12 +36,13 @@ class TestSearchIssues(unittest.TestCase): module to mock the GitHub API and test the function in isolation. Methods: - test_search_issues: Test that search_issues returns the correct issues. + test_search_issues_with_owner_and_repository: Test that search_issues with owner/repo returns the correct issues. + test_search_issues_with_just_owner_or_org: Test that search_issues with just an owner/org returns the correct issues. """ - def test_search_issues(self): - """Test that search_issues returns the correct issues.""" + def test_search_issues_with_owner_and_repository(self): + """Test that search_issues with owner/repo returns the correct issues.""" # Set up the mock GitHub connection object mock_issues = [ @@ -63,6 +64,30 @@ def test_search_issues(self): issues = search_issues("is:open", mock_connection, owners_and_repositories) self.assertEqual(issues, mock_issues) + def test_search_issues_with_just_owner_or_org(self): + """Test that search_issues with just an owner/org returns the correct issues.""" + + # Set up the mock GitHub connection object + mock_issues = [ + MagicMock(title="Issue 1"), + MagicMock(title="Issue 2"), + MagicMock(title="Issue 3"), + ] + + # simulating github3.structs.SearchIterator return value + mock_search_result = MagicMock() + mock_search_result.__iter__.return_value = iter(mock_issues) + mock_search_result.ratelimit_remaining = 30 + + mock_connection = MagicMock() + mock_connection.search_issues.return_value = mock_search_result + + # Call search_issues and check that it returns the correct issues + org = {"owner": "org1"} + owners = [org] + issues = search_issues("is:open", mock_connection, owners) + self.assertEqual(issues, mock_issues) + class TestGetOwnerAndRepository(unittest.TestCase): """Unit tests for the get_owners_and_repositories function. @@ -84,7 +109,7 @@ def test_get_owners_with_owner_and_repo_in_query(self): self.assertEqual(result[0].get("owner"), "owner1") self.assertEqual(result[0].get("repository"), "repo1") - def test_get_owner_and_repositories_with_repo_in_query(self): + def test_get_owner_and_repositories_without_repo_in_query(self): """Test get just owner.""" result = get_owners_and_repositories("org:owner1") self.assertEqual(result[0].get("owner"), "owner1") diff --git a/time_to_close.py b/time_to_close.py index a32262d..7efecb2 100644 --- a/time_to_close.py +++ b/time_to_close.py @@ -75,6 +75,7 @@ def get_stats_time_to_close( # Calculate the total time to close for all issues close_times = [] + total_time_to_close = None if issues_with_time_to_close: total_time_to_close = 0 for issue in issues_with_time_to_close: