From b42c5cccf36bf1d2e38e3e3d6138f92864fa5548 Mon Sep 17 00:00:00 2001 From: ananta <12180395+ananta@users.noreply.github.com> Date: Sat, 2 Dec 2023 15:18:09 -0500 Subject: [PATCH] refactor(get_env_vars): use get_env_vars() for consistent env variable retrieval re #168 --- config.py | 93 +++++++++++++++++++++++++++++++++++++++++ issue_metrics.py | 57 +++++++------------------ markdown_writer.py | 15 ++++--- test_issue_metrics.py | 22 +++++----- test_markdown_writer.py | 32 +++++++------- 5 files changed, 146 insertions(+), 73 deletions(-) create mode 100644 config.py diff --git a/config.py b/config.py new file mode 100644 index 0000000..a1f0542 --- /dev/null +++ b/config.py @@ -0,0 +1,93 @@ +"""A module for managing environment variables used in GitHub metrics calculation. + +This module defines a class for encapsulating environment variables and a function to retrieve these variables. + +Classes: + EnvVars: Represents the collection of environment variables used in the script. + +Functions: + get_env_vars: Retrieves and returns an instance of EnvVars populated with environment variables. +""" +import os +from typing import List + + +class EnvVars: + # pylint: disable=too-many-instance-attributes + """ + Environment variables + + Attributes: + search_query (str): Search query used to filter issues/prs/discussions on GitHub + gh_token (str): GitHub personal access token (PAT) for API authentication + labels_to_measure (List[str]): List of labels to measure how much time the lable is applied + ignore_users (List[str]): List of usernames to ignore when calculating metrics + github_server_url (str): URL of GitHub server (Github.com or Github Enterprise) + hide_author (str): If set, the author's information is hidden in the output + hide_time_to_first_response (str): If set, the time to first response metric is hidden in the output + hide_time_to_close (str): If set, the time to close metric is hidden in the output + hide_time_to_answer (str): If set, the time to answer discussions is hidden in the output + hide_label_metrics (str): If set, the label metrics are hidden in the output + """ + def __init__(self, search_query: str, gh_token: str, labels_to_measure: List[str], ignore_user: List[str], + github_server_url: str, hide_author: str, hide_time_to_first_response: str, + hide_time_to_close: str, hide_time_to_answer: str, hide_label_metrics: str): + self.search_query = search_query + self.gh_token = gh_token + self.labels_to_measure = labels_to_measure + self.ignore_users = ignore_user + self.github_server_url = github_server_url + self.hide_author = hide_author + self.hide_time_to_first_response = hide_time_to_first_response + self.hide_time_to_close = hide_time_to_close + self.hide_time_to_answer = hide_time_to_answer + self.hide_label_metrics = hide_label_metrics + + +def get_env_vars() -> EnvVars: + """ + Get the environment variables for use in the script. + + Returns EnvVars object with all environment variables + """ + search_query = os.getenv("SEARCH_QUERY") + if not search_query: + raise ValueError("SEARCH_QUERY environment variable not set") + + gh_token = os.getenv("GH_TOKEN") + if not gh_token: + raise ValueError("GITHUB_TOKEN environment variable not set") + + labels_to_measure = os.getenv("LABELS_TO_MEASURE") + if labels_to_measure: + labels_to_measure = labels_to_measure.split(",") + else: + labels_to_measure = [] + + ignore_users = os.getenv("IGNORE_USERS") + if ignore_users: + ignore_users = ignore_users.split(",") + else: + ignore_users = [] + + github_server_url = os.getenv("GITHUB_SERVER_URL") + + # Hidden columns + hide_author = os.getenv("HIDE_AUTHOR") + hide_time_to_first_response = os.getenv("HIDE_TIME_TO_FIRST_RESPONSE") + hide_time_to_close = os.getenv("HIDE_TIME_TO_CLOSE") + hide_time_to_answer = os.getenv("HIDE_TIME_TO_ANSWER") + hide_label_metrics = os.getenv("HIDE_LABEL_METRICS") + + return EnvVars( + search_query, + gh_token, + labels_to_measure, + ignore_users, + github_server_url, + hide_author, + hide_time_to_first_response, + hide_time_to_close, + hide_time_to_answer, + hide_label_metrics + ) diff --git a/issue_metrics.py b/issue_metrics.py index ef3c81b..3a62f05 100644 --- a/issue_metrics.py +++ b/issue_metrics.py @@ -6,7 +6,7 @@ their metrics to a markdown file. Functions: - get_env_vars() -> tuple[str, str]: Get the environment variables for use + get_env_vars() -> EnvVars: Get the environment variables for use in the script. search_issues(search_query: str, github_connection: github3.GitHub) -> github3.structs.SearchIterator: @@ -21,7 +21,6 @@ main(): Run the issue-metrics script. """ -import os from os.path import dirname, join import sys from typing import List, Union @@ -42,32 +41,7 @@ get_stats_time_to_first_response, measure_time_to_first_response, ) - - -def get_env_vars() -> tuple[str, str, List[str]]: - """ - Get the environment variables for use in the script. - - Returns: - str: the search query used to filter issues, prs, and discussions - str: the github token used to authenticate to github.com - List[str]: a list of users to ignore when calculating metrics - """ - search_query = os.getenv("SEARCH_QUERY") - if not search_query: - raise ValueError("SEARCH_QUERY environment variable not set") - - token = os.getenv("GH_TOKEN") - if not token: - raise ValueError("GITHUB_TOKEN environment variable not set") - - ignore_users = os.getenv("IGNORE_USERS") - if ignore_users: - ignore_users = ignore_users.split(",") - else: - ignore_users = [] - - return search_query, token, ignore_users +from config import get_env_vars def search_issues( @@ -123,17 +97,16 @@ def auth_to_github() -> github3.GitHub: Returns: github3.GitHub: A github api connection. """ - if token := os.getenv("GH_TOKEN"): - if not os.getenv("GITHUB_SERVER_URL"): - github_connection = github3.login(token=token) - elif os.getenv("GITHUB_SERVER_URL") == "https://github.com": - github_connection = github3.login(token=token) - else: - github_connection = github3.GitHubEnterprise( - os.getenv("GITHUB_SERVER_URL"), token=token - ) + env_vars = get_env_vars() + token = env_vars.gh_token + github_server_url = env_vars.github_server_url + + if github_server_url and github_server_url != "https://github.com": + github_connection = github3.GitHubEnterprise( + github_server_url, token=token + ) else: - raise ValueError("GH_TOKEN environment variable not set") + github_connection = github3.login(token=token) return github_connection # type: ignore @@ -269,9 +242,9 @@ def main(): # Get the environment variables for use in the script env_vars = get_env_vars() - search_query = env_vars[0] - token = env_vars[1] - ignore_users = env_vars[2] + search_query = env_vars.search_query + token = env_vars.gh_token + ignore_users = env_vars.ignore_users # Get the repository owner and name from the search query owner = get_owner(search_query) @@ -284,7 +257,7 @@ def main(): ) # Determine if there are label to measure - labels = os.environ.get("LABELS_TO_MEASURE") + labels = env_vars.labels_to_measure if labels: labels = labels.split(",") else: diff --git a/markdown_writer.py b/markdown_writer.py index d94cedb..1ae1714 100644 --- a/markdown_writer.py +++ b/markdown_writer.py @@ -24,11 +24,11 @@ Get the columns that are not hidden. """ -import os from datetime import timedelta from typing import List, Union from classes import IssueWithMetrics +from config import get_env_vars def get_non_hidden_columns(labels) -> List[str]: @@ -43,24 +43,27 @@ def get_non_hidden_columns(labels) -> List[str]: """ columns = ["Title", "URL"] + + env_vars = get_env_vars() + # Find the number of columns and which are to be hidden - hide_author = os.getenv("HIDE_AUTHOR") + hide_author = env_vars.hide_author if not hide_author: columns.append("Author") - hide_time_to_first_response = os.getenv("HIDE_TIME_TO_FIRST_RESPONSE") + hide_time_to_first_response = env_vars.hide_time_to_first_response if not hide_time_to_first_response: columns.append("Time to first response") - hide_time_to_close = os.getenv("HIDE_TIME_TO_CLOSE") + hide_time_to_close = env_vars.hide_time_to_close if not hide_time_to_close: columns.append("Time to close") - hide_time_to_answer = os.getenv("HIDE_TIME_TO_ANSWER") + hide_time_to_answer = env_vars.hide_time_to_answer if not hide_time_to_answer: columns.append("Time to answer") - hide_label_metrics = os.getenv("HIDE_LABEL_METRICS") + hide_label_metrics = env_vars.hide_label_metrics if not hide_label_metrics and labels: for label in labels: columns.append(f"Time spent in {label}") diff --git a/test_issue_metrics.py b/test_issue_metrics.py index fdedb9c..71e4908 100644 --- a/test_issue_metrics.py +++ b/test_issue_metrics.py @@ -60,6 +60,7 @@ class TestAuthToGithub(unittest.TestCase): """Test the auth_to_github function.""" @patch("github3.login") + @patch.dict(os.environ, {"GH_TOKEN": "test_token", "SEARCH_QUERY": "is:open repo:user/repo"}) def test_auth_to_github_with_token(self, mock_login): """Test that auth_to_github works with a token. @@ -72,9 +73,6 @@ def test_auth_to_github_with_token(self, mock_login): mock_gh = MagicMock() mock_login.return_value = mock_gh - # Set up the environment variable - os.environ["GH_TOKEN"] = "test_token" - # Call the function github_connection = auth_to_github() @@ -82,11 +80,9 @@ def test_auth_to_github_with_token(self, mock_login): self.assertEqual(github_connection, mock_gh) mock_login.assert_called_once_with(token="test_token") + @patch.dict(os.environ, {}, clear=True) def test_auth_to_github_no_token(self): """Test that auth_to_github raises a ValueError if GH_TOKEN is not set.""" - # Unset the GH_TOKEN environment variable - if "GH_TOKEN" in os.environ: - del os.environ["GH_TOKEN"] # Call auth_to_github and check that it raises a ValueError with self.assertRaises(ValueError): @@ -96,15 +92,17 @@ def test_auth_to_github_no_token(self): class TestGetEnvVars(unittest.TestCase): """Test suite for the get_env_vars function.""" + @patch.dict(os.environ, {"GH_TOKEN": "test_token", "SEARCH_QUERY": "is:issue is:open repo:user/repo"}) def test_get_env_vars(self): """Test that the function correctly retrieves the environment variables.""" - # Set the environment variables - os.environ["SEARCH_QUERY"] = "is:issue is:open repo:org/repo" # Call the function and check the result - result = get_env_vars() - expected_result = "is:issue is:open repo:org/repo" - self.assertEqual(result[0], expected_result) + search_query = get_env_vars().search_query + gh_token = get_env_vars().gh_token + gh_token_expected_result = "test_token" + search_query_expected_result = "is:issue is:open repo:user/repo" + self.assertEqual(gh_token, gh_token_expected_result) + self.assertEqual(search_query, search_query_expected_result) def test_get_env_vars_missing_query(self): """Test that the function raises a ValueError @@ -138,6 +136,7 @@ class TestMain(unittest.TestCase): os.environ, { "SEARCH_QUERY": "is:open repo:user/repo", + "GH_TOKEN": "test_token", }, ) def test_main( @@ -198,6 +197,7 @@ def test_main( os.environ, { "SEARCH_QUERY": "is:open repo:org/repo", + "GH_TOKEN": "test_token", }, ) def test_main_no_issues_found( diff --git a/test_markdown_writer.py b/test_markdown_writer.py index f741e66..38d4025 100644 --- a/test_markdown_writer.py +++ b/test_markdown_writer.py @@ -15,6 +15,13 @@ from markdown_writer import write_to_markdown +@patch.dict( + os.environ, + { + "SEARCH_QUERY": "is:open repo:user/repo", + "GH_TOKEN": "test_token" + }, +) class TestWriteToMarkdown(unittest.TestCase): """Test the write_to_markdown function.""" @@ -234,23 +241,20 @@ def test_write_to_markdown_no_issues(self): ) +@patch.dict( + os.environ, + { + "SEARCH_QUERY": "is:open repo:user/repo", + "GH_TOKEN": "test_token", + "HIDE_TIME_TO_FIRST_RESPONSE": "True", + "HIDE_TIME_TO_CLOSE": "True", + "HIDE_TIME_TO_ANSWER": "True", + "HIDE_LABEL_METRICS": "True" + }, +) class TestWriteToMarkdownWithEnv(unittest.TestCase): """Test the write_to_markdown function with the HIDE* environment variables set.""" - def setUp(self): - # Set the HIDE* environment variables to True - os.environ["HIDE_TIME_TO_FIRST_RESPONSE"] = "True" - os.environ["HIDE_TIME_TO_CLOSE"] = "True" - os.environ["HIDE_TIME_TO_ANSWER"] = "True" - os.environ["HIDE_LABEL_METRICS"] = "True" - - def tearDown(self): - # Unset the HIDE* environment variables - os.environ.pop("HIDE_TIME_TO_FIRST_RESPONSE") - os.environ.pop("HIDE_TIME_TO_CLOSE") - os.environ.pop("HIDE_TIME_TO_ANSWER") - os.environ.pop("HIDE_LABEL_METRICS") - def test_writes_markdown_file_with_non_hidden_columns_only(self): """ Test that write_to_markdown writes the correct