From feb8b568e4ce3f0c721d938a825b7992073e5a8e Mon Sep 17 00:00:00 2001 From: jmeridth Date: Sun, 30 Mar 2025 17:45:40 -0500 Subject: [PATCH 1/2] feat: extract environment variable handling - [x] central env.py for gathering and setting env vars - [x] bool and int env var handling and tests broken out Signed-off-by: jmeridth --- env.py | 145 ++++++++++++++++++++++++++++++++++++ stale_repos.py | 46 +++--------- test_env.py | 173 +++++++++++++++++++++++++++++++++++++++++++ test_env_get_bool.py | 81 ++++++++++++++++++++ test_env_get_int.py | 40 ++++++++++ test_stale_repos.py | 44 ----------- 6 files changed, 450 insertions(+), 79 deletions(-) create mode 100644 env.py create mode 100644 test_env.py create mode 100644 test_env_get_bool.py create mode 100644 test_env_get_int.py diff --git a/env.py b/env.py new file mode 100644 index 0000000..fef0bfc --- /dev/null +++ b/env.py @@ -0,0 +1,145 @@ +"""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. + +Functions: + get_bool_env_var: Gets a boolean environment variable. +""" + +import os +from os.path import dirname, join + +from dotenv import load_dotenv + + +class EnvVars: + # pylint: disable=too-many-instance-attributes, too-few-public-methods + """ + Environment variables + + Attributes: + gh_app_id (int | None): The GitHub App ID to use for authentication + gh_app_installation_id (int | None): The GitHub App Installation ID to use for + authentication + gh_app_private_key_bytes (bytes): The GitHub App Private Key as bytes to use for + authentication + gh_token (str | None): GitHub personal access token (PAT) for API authentication + ghe (str): The GitHub Enterprise URL to use for authentication + skip_empty_reports (bool): If true, Skips report creation when no stale + repositories are identified + """ + + def __init__( + self, + gh_app_id: int | None, + gh_app_installation_id: int | None, + gh_app_private_key_bytes: bytes, + gh_app_enterprise_only: bool, + gh_token: str | None, + ghe: str | None, + skip_empty_reports: bool, + ): + self.gh_app_id = gh_app_id + self.gh_app_installation_id = gh_app_installation_id + self.gh_app_private_key_bytes = gh_app_private_key_bytes + self.gh_app_enterprise_only = gh_app_enterprise_only + self.gh_token = gh_token + self.ghe = ghe + self.skip_empty_reports = skip_empty_reports + + def __repr__(self): + return ( + f"EnvVars(" + f"{self.gh_app_id}," + f"{self.gh_app_installation_id}," + f"{self.gh_app_private_key_bytes}," + f"{self.gh_app_enterprise_only}," + f"{self.gh_token}," + f"{self.ghe}," + f"{self.skip_empty_reports}," + ) + + +def get_bool_env_var(env_var_name: str, default: bool = False) -> bool: + """Get a boolean environment variable. + + Args: + env_var_name: The name of the environment variable to retrieve. + default: The default value to return if the environment variable is not set. + + Returns: + The value of the environment variable as a boolean. + """ + ev = os.environ.get(env_var_name, "") + if ev == "" and default: + return default + return ev.strip().lower() == "true" + + +def get_int_env_var(env_var_name: str) -> int | None: + """Get an integer environment variable. + + Args: + env_var_name: The name of the environment variable to retrieve. + + Returns: + The value of the environment variable as an integer or None. + """ + env_var = os.environ.get(env_var_name) + if env_var is None or not env_var.strip(): + return None + try: + return int(env_var) + except ValueError: + return None + + +def get_env_vars( + test: bool = False, +) -> EnvVars: + """ + get_env_vars: Gets the environment variables. + + Args: + test: True if this is a test run; False otherwise. + + Returns: + The environment variables. + """ + + if not test: + # Load from .env file if it exists and not testing + dotenv_path = join(dirname(__file__), ".env") + load_dotenv(dotenv_path) + + gh_token = os.getenv("GH_TOKEN", "") + gh_app_id = get_int_env_var("GH_APP_ID") + gh_app_installation_id = get_int_env_var("GH_APP_INSTALLATION_ID") + gh_app_private_key_bytes = os.getenv("GH_APP_PRIVATE_KEY", "").encode("utf8") + ghe = os.getenv("GH_ENTERPRISE_URL") + gh_app_enterprise_only = get_bool_env_var("GITHUB_APP_ENTERPRISE_ONLY") + skip_empty_reports = get_bool_env_var("SKIP_EMPTY_REPORTS", True) + + if gh_app_id and (not gh_app_private_key_bytes or not gh_app_installation_id): + raise ValueError( + "GH_APP_ID set and GH_APP_INSTALLATION_ID or GH_APP_PRIVATE_KEY variable not set" + ) + + if ( + not gh_app_id + and not gh_app_private_key_bytes + and not gh_app_installation_id + and not gh_token + ): + raise ValueError("GH_TOKEN environment variable not set") + + return EnvVars( + gh_app_id, + gh_app_installation_id, + gh_app_private_key_bytes, + gh_app_enterprise_only, + gh_token, + ghe, + skip_empty_reports, + ) diff --git a/stale_repos.py b/stale_repos.py index 1bdd957..61f8c05 100755 --- a/stale_repos.py +++ b/stale_repos.py @@ -5,11 +5,10 @@ import json import os from datetime import datetime, timezone -from os.path import dirname, join import github3 from dateutil.parser import parse -from dotenv import load_dotenv +from env import get_env_vars import auth @@ -31,26 +30,21 @@ def main(): # pragma: no cover """ print("Starting stale repo search...") - # Load env variables from file - dotenv_path = join(dirname(__file__), ".env") - load_dotenv(dotenv_path) - - token = os.getenv("GH_TOKEN") - gh_app_id = os.getenv("GH_APP_ID") - gh_app_installation_id = os.getenv("GH_APP_INSTALLATION_ID") - gh_app_private_key = os.getenv("GH_APP_PRIVATE_KEY").encode("utf8") - ghe = os.getenv("GH_ENTERPRISE_URL") - gh_app_enterprise_only = os.getenv("GITHUB_APP_ENTERPRISE_ONLY") - skip_empty_reports = ( - False if os.getenv("SKIP_EMPTY_REPORTS").lower() == "false" else True - ) + env_vars = get_env_vars() + token = env_vars.gh_token + gh_app_id = env_vars.gh_app_id + gh_app_installation_id = env_vars.gh_app_installation_id + gh_app_private_key_bytes = env_vars.gh_app_private_key_bytes + ghe = env_vars.ghe + gh_app_enterprise_only = env_vars.gh_app_enterprise_only + skip_empty_reports = env_vars.skip_empty_reports # Auth to GitHub.com or GHE github_connection = auth.auth_to_github( token, gh_app_id, gh_app_installation_id, - gh_app_private_key, + gh_app_private_key_bytes, ghe, gh_app_enterprise_only, ) @@ -221,7 +215,7 @@ def get_active_date(repo): commit = repo.branch(repo.default_branch).commit active_date = parse(commit.commit.as_dict()["committer"]["date"]) elif activity_method == "pushed": - last_push_str = repo.pushed_at # type: ignored + last_push_str = repo.pushed_at # type: ignore if last_push_str is None: return None active_date = parse(last_push_str) @@ -348,24 +342,6 @@ def output_to_json(inactive_repos, file=None): return inactive_repos_json -def get_int_env_var(env_var_name): - """Get an integer environment variable. - - Args: - env_var_name: The name of the environment variable to retrieve. - - Returns: - The value of the environment variable as an integer or None. - """ - env_var = os.environ.get(env_var_name) - if env_var is None or not env_var.strip(): - return None - try: - return int(env_var) - except ValueError: - return None - - def set_repo_data( repo, days_inactive, active_date_disp, visibility, additional_metrics ): diff --git a/test_env.py b/test_env.py new file mode 100644 index 0000000..f1fbd4e --- /dev/null +++ b/test_env.py @@ -0,0 +1,173 @@ +"""A module containing unit tests for the config module functions. + +Classes: + TestGetIntFromEnv: A class to test the get_int_env_var function. + TestEnvVars: A class to test the get_env_vars function. + +""" + +import os +import unittest +from unittest.mock import patch + +from env import EnvVars, get_env_vars + +TOKEN = "test_token" + + +class TestGetEnvVars(unittest.TestCase): + """ + Test suite for the get_env_vars function. + """ + + def setUp(self): + env_keys = [ + "GH_APP_ID", + "GH_APP_INSTALLATION_ID", + "GH_APP_PRIVATE_KEY", + "GH_TOKEN", + "GHE", + "SKIP_EMPTY_REPORTS", + ] + for key in env_keys: + if key in os.environ: + del os.environ[key] + + @patch.dict( + os.environ, + { + "GH_APP_ID": "12345", + "GH_APP_INSTALLATION_ID": "678910", + "GH_APP_PRIVATE_KEY": "hello", + "GH_TOKEN": "", + "GH_ENTERPRISE_URL": "", + }, + clear=True, + ) + def test_get_env_vars_with_github_app(self): + """Test that all environment variables are set correctly using GitHub App""" + expected_result = EnvVars( + gh_app_id=12345, + gh_app_installation_id=678910, + gh_app_private_key_bytes=b"hello", + gh_app_enterprise_only=False, + gh_token="", + ghe="", + skip_empty_reports=True, + ) + result = get_env_vars(True) + self.assertEqual(str(result), str(expected_result)) + + @patch.dict( + os.environ, + { + "GH_APP_ID": "", + "GH_APP_INSTALLATION_ID": "", + "GH_APP_PRIVATE_KEY": "", + "GH_ENTERPRISE_URL": "", + "GH_TOKEN": TOKEN, + }, + clear=True, + ) + def test_get_env_vars_with_token(self): + """Test that all environment variables are set correctly using a list of repositories""" + expected_result = EnvVars( + gh_app_id=None, + gh_app_installation_id=None, + gh_app_private_key_bytes=b"", + gh_app_enterprise_only=False, + gh_token=TOKEN, + ghe="", + skip_empty_reports=True, + ) + result = get_env_vars(True) + self.assertEqual(str(result), str(expected_result)) + + @patch.dict( + os.environ, + { + "GH_APP_ID": "", + "GH_APP_INSTALLATION_ID": "", + "GH_APP_PRIVATE_KEY": "", + "GH_TOKEN": "", + }, + clear=True, + ) + def test_get_env_vars_missing_token(self): + """Test that an error is raised if the TOKEN environment variables is not set""" + with self.assertRaises(ValueError): + get_env_vars(True) + + @patch.dict( + os.environ, + { + "GH_APP_ID": "", + "GH_APP_INSTALLATION_ID": "", + "GH_APP_PRIVATE_KEY": "", + "GH_TOKEN": TOKEN, + "GH_ENTERPRISE_URL": "", + "SKIP_EMPTY_REPORTS": "false", + }, + ) + def test_get_env_vars_optional_values(self): + """Test that optional values are set when provided""" + expected_result = EnvVars( + gh_app_id=None, + gh_app_installation_id=None, + gh_app_private_key_bytes=b"", + gh_app_enterprise_only=False, + gh_token=TOKEN, + ghe="", + skip_empty_reports=False, + ) + result = get_env_vars(True) + self.assertEqual(str(result), str(expected_result)) + + @patch.dict( + os.environ, + { + "GH_APP_ID": "", + "GH_APP_INSTALLATION_ID": "", + "GH_APP_PRIVATE_KEY": "", + "GH_TOKEN": "TOKEN", + }, + clear=True, + ) + def test_get_env_vars_optionals_are_defaulted(self): + """Test that optional values are set to their default values if not provided""" + expected_result = EnvVars( + gh_app_id=None, + gh_app_installation_id=None, + gh_app_private_key_bytes=b"", + gh_app_enterprise_only=False, + gh_token="TOKEN", + ghe=None, + skip_empty_reports=True, + ) + result = get_env_vars(True) + self.assertEqual(str(result), str(expected_result)) + + @patch.dict( + os.environ, + { + "ORGANIZATION": "my_organization", + "GH_APP_ID": "12345", + "GH_APP_INSTALLATION_ID": "", + "GH_APP_PRIVATE_KEY": "", + "GH_TOKEN": "", + }, + clear=True, + ) + def test_get_env_vars_auth_with_github_app_installation_missing_inputs(self): + """Test that an error is raised there are missing inputs for the gh app""" + with self.assertRaises(ValueError) as context_manager: + get_env_vars(True) + the_exception = context_manager.exception + self.assertEqual( + str(the_exception), + "GH_APP_ID set and GH_APP_INSTALLATION_ID or GH_APP_PRIVATE_KEY variable not set", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/test_env_get_bool.py b/test_env_get_bool.py new file mode 100644 index 0000000..7491e9c --- /dev/null +++ b/test_env_get_bool.py @@ -0,0 +1,81 @@ +"""Test the get_bool_env_var function""" + +import os +import unittest +from unittest.mock import patch + +from env import get_bool_env_var + + +class TestEnvGetBool(unittest.TestCase): + """Test the get_bool_env_var function""" + + @patch.dict( + os.environ, + { + "TEST_BOOL": "true", + }, + clear=True, + ) + def test_get_bool_env_var_that_exists_and_is_true(self): + """Test that gets a boolean environment variable that exists and is true""" + result = get_bool_env_var("TEST_BOOL") + self.assertTrue(result) + + @patch.dict( + os.environ, + { + "TEST_BOOL": "false", + }, + clear=True, + ) + def test_get_bool_env_var_that_exists_and_is_false(self): + """Test that gets a boolean environment variable that exists and is false""" + result = get_bool_env_var("TEST_BOOL") + self.assertFalse(result) + + @patch.dict( + os.environ, + { + "TEST_BOOL": "nope", + }, + clear=True, + ) + def test_get_bool_env_var_that_exists_and_is_false_due_to_invalid_value(self): + """Test that gets a boolean environment variable that exists and is false + due to an invalid value + """ + result = get_bool_env_var("TEST_BOOL", False) + self.assertFalse(result) + + @patch.dict( + os.environ, + { + "TEST_BOOL": "false", + }, + clear=True, + ) + def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_true(self): + """Test that gets a boolean environment variable that does not exist + and default value returns: true + """ + result = get_bool_env_var("DOES_NOT_EXIST", True) + self.assertTrue(result) + + @patch.dict( + os.environ, + { + "TEST_BOOL": "true", + }, + clear=True, + ) + def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_false(self): + """Test that gets a boolean environment variable that does not exist + and default value returns: false + """ + result = get_bool_env_var("DOES_NOT_EXIST", False) + self.assertFalse(result) + + +if __name__ == "__main__": + unittest.main() diff --git a/test_env_get_int.py b/test_env_get_int.py new file mode 100644 index 0000000..83d3484 --- /dev/null +++ b/test_env_get_int.py @@ -0,0 +1,40 @@ +"""Test the get_int_env_var function""" + +import os +import unittest +from unittest.mock import patch + +from env import get_int_env_var + + +class TestEnvGetInt(unittest.TestCase): + """Test the get_int_env_var function""" + + @patch.dict(os.environ, {"TEST_INT": "1234"}, clear=True) + def test_get_int_env_var_that_exists_and_has_value(self): + """Test that gets an integer environment variable that exists and has value""" + result = get_int_env_var("TEST_INT") + self.assertEqual(result, 1234) + + @patch.dict(os.environ, {"TEST_INT": "nope"}, clear=True) + def test_get_int_env_var_that_exists_and_is_none_due_to_invalid_value(self): + """Test that gets None due to an invalid value""" + result = get_int_env_var("TEST_INT") + self.assertIsNone(result) + + @patch.dict(os.environ, {"TEST_INT": ""}, clear=True) + def test_get_int_env_var_that_exists_and_is_none_due_to_empty_string(self): + """Test that gets None due to an empty string""" + result = get_int_env_var("TEST_INT") + self.assertIsNone(result) + + def test_get_int_env_var_that_does_not_exist_and_default_value_returns_none(self): + """Test that gets a integer environment variable that does not exist + and default value returns: none + """ + result = get_int_env_var("DOES_NOT_EXIST") + self.assertIsNone(result) + + +if __name__ == "__main__": + unittest.main() diff --git a/test_stale_repos.py b/test_stale_repos.py index d76ccc9..3e7c41d 100644 --- a/test_stale_repos.py +++ b/test_stale_repos.py @@ -29,56 +29,12 @@ get_days_since_last_pr, get_days_since_last_release, get_inactive_repos, - get_int_env_var, is_repo_exempt, output_to_json, write_to_markdown, ) -class TestGetIntFromEnv(unittest.TestCase): - """ - Test suite for the get_int_from_env function. - - ... - - Test methods: - - test_get_int_env_var: Test returns the expected integer value. - - test_get_int_env_var_with_empty_env_var: Test returns None when environment variable - is empty. - - test_get_int_env_var_with_non_integer: Test returns None when environment variable - is a non-integer. - """ - - @patch.dict(os.environ, {"INT_ENV_VAR": "12345"}) - def test_get_int_env_var(self): - """ - Test that get_int_env_var returns the expected integer value. - """ - result = get_int_env_var("INT_ENV_VAR") - self.assertEqual(result, 12345) - - @patch.dict(os.environ, {"INT_ENV_VAR": ""}) - def test_get_int_env_var_with_empty_env_var(self): - """ - This test verifies that the get_int_env_var function returns None - when the environment variable is empty. - - """ - result = get_int_env_var("INT_ENV_VAR") - self.assertIsNone(result) - - @patch.dict(os.environ, {"INT_ENV_VAR": "not_an_int"}) - def test_get_int_env_var_with_non_integer(self): - """ - Test that get_int_env_var returns None when the environment variable is - a non-integer. - - """ - result = get_int_env_var("INT_ENV_VAR") - self.assertIsNone(result) - - class GetInactiveReposTestCase(unittest.TestCase): """ Unit test case for the get_inactive_repos() function. From c6ef210689dc573e08a1d0057e52c442b5dbe87b Mon Sep 17 00:00:00 2001 From: jmeridth Date: Sun, 30 Mar 2025 18:00:22 -0500 Subject: [PATCH 2/2] fix: a vs an, integer thanks copilot Signed-off-by: jmeridth --- test_env_get_int.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_env_get_int.py b/test_env_get_int.py index 83d3484..a820b8c 100644 --- a/test_env_get_int.py +++ b/test_env_get_int.py @@ -29,7 +29,7 @@ def test_get_int_env_var_that_exists_and_is_none_due_to_empty_string(self): self.assertIsNone(result) def test_get_int_env_var_that_does_not_exist_and_default_value_returns_none(self): - """Test that gets a integer environment variable that does not exist + """Test that gets an integer environment variable that does not exist and default value returns: none """ result = get_int_env_var("DOES_NOT_EXIST")