diff --git a/cloud-chatops/.gitignore b/cloud-chatops/.gitignore index 0ccb889..03d3758 100644 --- a/cloud-chatops/.gitignore +++ b/cloud-chatops/.gitignore @@ -1,3 +1,5 @@ .idea/ __pycache__/ tests/__pycache__/ +.coverage +coverage.xml diff --git a/cloud-chatops/requirements.txt b/cloud-chatops/requirements.txt index 59fd58e..de3ba2e 100644 --- a/cloud-chatops/requirements.txt +++ b/cloud-chatops/requirements.txt @@ -7,4 +7,5 @@ python-dateutil pytest pytest-cov pylint -black \ No newline at end of file +black +pyyaml \ No newline at end of file diff --git a/cloud-chatops/src/errors.py b/cloud-chatops/src/errors.py index a4f9aff..3bb084f 100644 --- a/cloud-chatops/src/errors.py +++ b/cloud-chatops/src/errors.py @@ -10,7 +10,7 @@ class UnknownHTTPError(RuntimeError): class RepositoriesNotGiven(RuntimeError): - """Error: repos.csv does not contain any repositories.""" + """Error: repos supplied is empty does not contain any repositories.""" class TokensNotGiven(RuntimeError): diff --git a/cloud-chatops/src/features/base_feature.py b/cloud-chatops/src/features/base_feature.py index 5c8ecec..90aef73 100644 --- a/cloud-chatops/src/features/base_feature.py +++ b/cloud-chatops/src/features/base_feature.py @@ -10,7 +10,7 @@ from slack_sdk import WebClient from slack_sdk.errors import SlackApiError from dateutil import parser as datetime_parser -from read_data import get_token, get_repos, get_user_map +from read_data import get_token, get_config from get_github_prs import GetGitHubPRs from pr_dataclass import PrData from errors import FailedToPostMessage, UserNotFound @@ -33,8 +33,8 @@ class BaseFeature(ABC): def __init__(self): self.channel = DEFAULT_CHANNEL self.client = WebClient(token=get_token("SLACK_BOT_TOKEN")) - self.prs = GetGitHubPRs(get_repos(), DEFAULT_REPO_OWNER).run() - self.slack_ids = get_user_map() + self.prs = GetGitHubPRs(get_config("repos")).run() + self.slack_ids = get_config("user-map") def _post_reminder_message(self) -> str: """ @@ -175,7 +175,7 @@ def check_pr(self, info: PrData) -> PrData: :param info: The information to validate. :return: The validated information. """ - slack_ids = get_user_map() + slack_ids = get_config("user-map") new_info = replace( info, user=slack_ids.get(info.user, info.user), diff --git a/cloud-chatops/src/get_github_prs.py b/cloud-chatops/src/get_github_prs.py index 0c77505..489c27c 100644 --- a/cloud-chatops/src/get_github_prs.py +++ b/cloud-chatops/src/get_github_prs.py @@ -21,14 +21,13 @@ class GetGitHubPRs: This class handles getting the open PRs from the GitHub Rest API. """ - def __init__(self, repos: List[str], owner: str): + def __init__(self, repos: Dict[str, List]): """ This method initialises the class with the following attributes. :param repos: A list of repositories to get pull requests for. :param repos: The owner of the above repositories. """ self.repos = repos - self.owner = owner self._http_handler = HTTPHandler() def run(self) -> List[PrData]: @@ -37,17 +36,21 @@ def run(self) -> List[PrData]: It runs the HTTP request methods and returns the responses. :return: The responses from the HTTP requests. """ - responses = self._request_all_repos_http() + responses = [] + for owner in self.repos: + responses += self._request_all_repos_http(owner, self.repos.get(owner)) return self._parse_pr_to_dataclass(responses) - def _request_all_repos_http(self) -> List[Dict]: + def _request_all_repos_http(self, owner: str, repos: List[str]) -> List[Dict]: """ This method starts a request for each repository and returns a list of those PRs. - :return: A dictionary of repos and their PRs. + :param owner: The organisation for repos + :param repos: List of repository names + :return: A list of PRs stored as dictionaries """ responses = [] - for repo in self.repos: - url = f"https://api.github.com/repos/{self.owner}/{repo}/pulls" + for repo in repos: + url = f"https://api.github.com/repos/{owner}/{repo}/pulls" responses += self._http_handler.make_request(url) return responses diff --git a/cloud-chatops/src/main.py b/cloud-chatops/src/main.py index 1339d93..d5a5146 100644 --- a/cloud-chatops/src/main.py +++ b/cloud-chatops/src/main.py @@ -10,7 +10,7 @@ import schedule from features.pr_reminder import PostPRsToSlack from features.post_to_dms import PostToDMs -from read_data import get_token, validate_required_files, get_user_map +from read_data import get_token, validate_required_files, get_config PULL_REQUESTS_CHANNEL = "C03RT2F6WHZ" @@ -60,7 +60,7 @@ def run_global_reminder(channel) -> None: def run_personal_reminder() -> None: """This is a placeholder function for the schedule to accept.""" - users = list(get_user_map().values()) + users = list(get_config("user-map").values()) PostToDMs().run(users=users, post_all=False) schedule.every().monday.at("09:00").do( diff --git a/cloud-chatops/src/read_data.py b/cloud-chatops/src/read_data.py index c6ed7ce..d713fd3 100644 --- a/cloud-chatops/src/read_data.py +++ b/cloud-chatops/src/read_data.py @@ -1,9 +1,10 @@ """This module handles reading data from files such as secrets and user maps.""" -from typing import List, Dict +from typing import Dict, Union import sys import os import json +import yaml from errors import ( RepositoriesNotGiven, UserMapNotGiven, @@ -13,13 +14,19 @@ # Production secret path PATH = "/usr/src/app/cloud_chatops_secrets/" try: - if sys.argv[1] == "local": + if sys.argv[1] == "dev" or sys.argv[1] == "development": # Using dev secrets here for local testing as it runs the application # in a separate Slack Workspace than the production application. # This means the slash commands won't be picked up by the production application. PATH = f"{os.environ['HOME']}/dev_cloud_chatops_secrets/" + + elif sys.argv[1] == "prod" or sys.argv[1] == "production": + # Using prod secrets here in case the application is run directly on host without Docker. + PATH = f"{os.environ['HOME']}/cloud_chatops_secrets/" + except IndexError: pass + except KeyError: print( "Are you trying to run locally? Couldn't find HOME in your environment variables." @@ -31,9 +38,9 @@ def validate_required_files() -> None: """ This function checks that all required files have data in them before the application runs. """ - repos = get_repos() + repos = get_config("repos") if not repos: - raise RepositoriesNotGiven("repos.csv does not contain any repositories.") + raise RepositoriesNotGiven("config.yml does not contain any repositories.") tokens = ["SLACK_BOT_TOKEN", "SLACK_APP_TOKEN", "GITHUB_TOKEN"] for token in tokens: @@ -42,12 +49,16 @@ def validate_required_files() -> None: raise TokensNotGiven( f"Token {token} does not have a value in secrets.json." ) - user_map = get_user_map() + user_map = get_config("user-map") if not user_map: - raise UserMapNotGiven("user_map.json is empty.") + raise UserMapNotGiven("config.yml does not contain a user map is empty.") for item, value in user_map.items(): if not value: raise UserMapNotGiven(f"User {item} does not have a Slack ID assigned.") + if not item: + raise UserMapNotGiven( + f"Slack member {value} does not have a GitHub username assigned." + ) def get_token(secret: str) -> str: @@ -62,37 +73,16 @@ def get_token(secret: str) -> str: return secrets[secret] -def get_repos() -> List[str]: +def get_config(section: str = "all") -> Union[Dict, str]: """ - This function reads the repo csv file and returns a list of repositories - :return: List of repositories as strings - """ - with open(PATH + "repos.csv", "r", encoding="utf-8") as file: - data = file.read() - repos = data.split(",") - if not repos[-1]: - repos = repos[:-1] - return repos - - -def get_user_map() -> Dict: + This function will return the specified section from the config file. + :param section: The section of the config to retrieve. + :return: The data retrieved from the config file. """ - This function gets the GitHub to Slack username mapping from the map file. - :return: Dictionary of username mapping - """ - with open(PATH + "user_map.json", "r", encoding="utf-8") as file: - data = file.read() - user_map = json.loads(data) - return user_map - - -def get_maintainer() -> str: - """ - This function will get the maintainer user's Slack ID from the text file. - :return: Slack Member ID - """ - with open(PATH + "maintainer.txt", "r", encoding="utf-8") as file: - data = file.read() - if not data: - return "U05RBU0RF4J" # Default Maintainer: Kalibh Halford - return data + with open(PATH + "config.yml", "r", encoding="utf-8") as config: + config_data = yaml.safe_load(config) + match section: + case "all": + return config_data + case _: + return config_data.get(section) diff --git a/cloud-chatops/tests/test_base_feature.py b/cloud-chatops/tests/test_base_feature.py index 9a935dc..04dc36d 100644 --- a/cloud-chatops/tests/test_base_feature.py +++ b/cloud-chatops/tests/test_base_feature.py @@ -10,18 +10,14 @@ @patch("features.base_feature.WebClient") @patch("features.base_feature.GetGitHubPRs") @patch("features.base_feature.get_token") -@patch("features.base_feature.get_repos") -@patch("features.base_feature.get_user_map") +@patch("features.base_feature.get_config") def instance_fixture( - mock_get_user_map, - mock_get_repos, + _, mock_get_token, mock_get_github_prs, mock_web_client, ): """This fixture provides a class instance for the tests""" - mock_get_user_map.return_value = {"mock_github": "mock_slack"} - mock_get_repos.return_value = ["mock_repo1", "mock_repo2"] mock_get_token.return_value = "mock_slack_token" mock_get_github_prs.run.return_value = [] mock_web_client.return_value = NonCallableMock() diff --git a/cloud-chatops/tests/test_get_github_prs.py b/cloud-chatops/tests/test_get_github_prs.py index 06e6987..65beeb5 100644 --- a/cloud-chatops/tests/test_get_github_prs.py +++ b/cloud-chatops/tests/test_get_github_prs.py @@ -11,9 +11,8 @@ @pytest.fixture(name="instance", scope="function") def instance_fixture(): """Creates a class fixture to use in the tests""" - mock_repos = ["repo1", "repo2"] - mock_owner = "mock_user" - return GetGitHubPRs(mock_repos, mock_owner) + mock_repos = {"owner1": ["repo1"]} + return GetGitHubPRs(mock_repos) @patch("get_github_prs.GetGitHubPRs._request_all_repos_http") @@ -21,35 +20,35 @@ def instance_fixture(): def test_run(mock_parse_pr_to_dataclass, mock_request_all_repos_http, instance): """Tests the run method returns the correct object""" res = instance.run() - mock_request_all_repos_http.assert_called_once_with() - mock_parse_pr_to_dataclass.assert_called_once_with( - mock_request_all_repos_http.return_value - ) + for owner in instance.repos: + mock_request_all_repos_http.assert_any_call(owner, instance.repos.get(owner)) + mock_parse_pr_to_dataclass.assert_called_once() assert res == mock_parse_pr_to_dataclass.return_value @patch("get_github_prs.HTTPHandler.make_request") def test_request_all_repos_http(mock_make_request, instance): """Test a request is made for each repo in the list""" + mock_owner = "owner1" + mock_repos = instance.repos.get(mock_owner) mock_make_request.side_effect = [ - [f"https://api.github.com/repos/{instance.owner}/{repo}/pulls"] - for repo in instance.repos + [f"https://api.github.com/repos/{mock_owner}/{repo}/pulls"] + for repo in mock_repos ] - res = instance._request_all_repos_http() - for repo in instance.repos: + res = instance._request_all_repos_http(mock_owner, mock_repos) + for repo in mock_repos: mock_make_request.assert_any_call( - f"https://api.github.com/repos/{instance.owner}/{repo}/pulls" + f"https://api.github.com/repos/{mock_owner}/{repo}/pulls" ) assert res == [ - f"https://api.github.com/repos/{instance.owner}/{repo}/pulls" - for repo in instance.repos + f"https://api.github.com/repos/{mock_owner}/{repo}/pulls" for repo in mock_repos ] def test_request_all_repos_http_none(instance): """Test that nothing is returned when no repos are given""" - instance.repos = [] - res = instance._request_all_repos_http() + instance.repos = {} + res = instance._request_all_repos_http("", []) assert res == [] diff --git a/cloud-chatops/tests/test_pr_message_builder.py b/cloud-chatops/tests/test_pr_message_builder.py index 037cedc..3735347 100644 --- a/cloud-chatops/tests/test_pr_message_builder.py +++ b/cloud-chatops/tests/test_pr_message_builder.py @@ -13,9 +13,8 @@ @patch("features.base_feature.WebClient") @patch("features.base_feature.GetGitHubPRs") @patch("features.base_feature.get_token") -@patch("features.base_feature.get_repos") -@patch("features.base_feature.get_user_map") -def instance_fixture(_, _2, _3, _4, _5): +@patch("features.base_feature.get_config") +def instance_fixture(_, _2, _3, _4): """Provides a class instance for the tests""" return PRMessageBuilder() @@ -128,8 +127,8 @@ def test_check_pr_age_old( @patch("features.base_feature.PRMessageBuilder._check_pr_age") -@patch("features.base_feature.get_user_map") -def test_check_pr_info_found_name(mock_user_map, mock_check_pr_age, instance): +@patch("features.base_feature.get_config") +def test_check_pr_info_found_name(mock_get_config, mock_check_pr_age, instance): """Test the dataclass is updated and name is found""" mock_data = PrData( pr_title="mock_title", @@ -139,16 +138,17 @@ def test_check_pr_info_found_name(mock_user_map, mock_check_pr_age, instance): draft=False, old=False, ) - mock_user_map.return_value = {"mock_github": "mock_slack"} + mock_get_config.return_value = {"mock_github": "mock_slack"} mock_check_pr_age.return_value = True res = instance.check_pr(mock_data) + mock_get_config.assert_called_once_with("user-map") assert res.user == "mock_slack" assert res.old @patch("features.base_feature.PRMessageBuilder._check_pr_age") -@patch("features.base_feature.get_user_map") -def test_check_pr_info_unfound_name(mock_user_map, _, instance): +@patch("features.base_feature.get_config") +def test_check_pr_info_unfound_name(mock_get_config, _, instance): """Test the dataclass is updated and name is not found""" mock_data = PrData( pr_title="mock_title", @@ -158,6 +158,7 @@ def test_check_pr_info_unfound_name(mock_user_map, _, instance): draft=False, old=False, ) - mock_user_map.return_value = {"mock_github": "mock_slack"} + mock_get_config.return_value = {"mock_github": "mock_slack"} res = instance.check_pr(mock_data) + mock_get_config.assert_called_once_with("user-map") assert res.user == "mock_user" diff --git a/cloud-chatops/tests/test_pr_reminder.py b/cloud-chatops/tests/test_pr_reminder.py index 2d472b9..e742d57 100644 --- a/cloud-chatops/tests/test_pr_reminder.py +++ b/cloud-chatops/tests/test_pr_reminder.py @@ -12,18 +12,14 @@ @patch("features.base_feature.WebClient") @patch("features.base_feature.GetGitHubPRs") @patch("features.base_feature.get_token") -@patch("features.base_feature.get_repos") -@patch("features.base_feature.get_user_map") +@patch("features.base_feature.get_config") def instance_fixture( - mock_get_user_map, - mock_get_repos, + _, mock_get_token, mock_get_github_prs, mock_web_client, ): """This fixture provides a class instance for the tests""" - mock_get_user_map.return_value = {"mock_github": "mock_slack"} - mock_get_repos.return_value = ["mock_repo1", "mock_repo2"] mock_get_token.return_value = "mock_slack_token" mock_get_github_prs.run.return_value = [] mock_web_client.return_value = NonCallableMock() diff --git a/cloud-chatops/tests/test_read_data.py b/cloud-chatops/tests/test_read_data.py index 72d5084..9ab44b2 100644 --- a/cloud-chatops/tests/test_read_data.py +++ b/cloud-chatops/tests/test_read_data.py @@ -1,7 +1,28 @@ """This test file covers all tests for the read_data module.""" - from unittest.mock import patch, mock_open -from read_data import get_token, get_repos, get_user_map, get_maintainer +import pytest +import yaml +from errors import RepositoriesNotGiven, TokensNotGiven, UserMapNotGiven +from read_data import get_token, get_config, validate_required_files + + +MOCK_CONFIG = """ +--- +maintainer: mock_maintainer +user-map: + mock_user_1: mock_id_1 + mock_user_2: mock_id_2 +repos: + organisation1: + - repo1 + - repo2 + organisation2: + - repo1 + - repo2 +defaults: + author: WX67YZ + channel: CH12NN34 +""" def test_get_token(): @@ -15,36 +36,95 @@ def test_get_token(): def test_get_user_map(): """This test ensures that the JSON file is read and converted to a dictionary correctly.""" - with patch("builtins.open", mock_open(read_data='{"mock_user_1": "mock_id_1"}')): - res = get_user_map() - assert res == {"mock_user_1": "mock_id_1"} + with patch("builtins.open", mock_open(read_data=MOCK_CONFIG)): + res = get_config("user-map") + assert res == {"mock_user_1": "mock_id_1", "mock_user_2": "mock_id_2"} def test_get_repos(): """This test checks that a list is returned if a string list of repos is read with no comma at the end.""" - with patch("builtins.open", mock_open(read_data="repo1,repo2")): - res = get_repos() - assert res == ["repo1", "repo2"] - - -def test_get_repos_trailing_separator(): - """ - This test checks that a list of repos is returned correctly if there is a trailing comma at the end of the list. - """ - with patch("builtins.open", mock_open(read_data="repo1,repo2,")): - res = get_repos() - assert res == ["repo1", "repo2"] + with patch("builtins.open", mock_open(read_data=MOCK_CONFIG)): + res = get_config("repos") + assert res == { + "organisation1": ["repo1", "repo2"], + "organisation2": ["repo1", "repo2"], + } def test_get_maintainer(): """This test checks that the user's name is returned.""" - with patch("builtins.open", mock_open(read_data="mock_person")): - res = get_maintainer() - assert res == "mock_person" + with patch("builtins.open", mock_open(read_data=MOCK_CONFIG)): + res = get_config("maintainer") + assert res == "mock_maintainer" + + +def test_get_config(): + """Test that the entire config is returned when no section is specified.""" + with patch("builtins.open", mock_open(read_data=MOCK_CONFIG)): + res = get_config() + assert res == yaml.safe_load(MOCK_CONFIG) + + +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["mock_bot", "mock_app", "mock_github"] + mock_get_config.side_effect = [{"owner1": ["repo1"]}, {"github1": "slack1"}] + validate_required_files() + + +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files_fail_repo(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["mock_bot", "mock_app", "mock_github"] + mock_get_config.side_effect = [{}, {"github1": "slack1"}] + with pytest.raises(RepositoriesNotGiven): + validate_required_files() + + +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files_fail_token(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["", "mock_app", "mock_github"] + mock_get_config.side_effect = [{"owner1": ["repo1"]}, {"github1": "slack1"}] + with pytest.raises(TokensNotGiven): + validate_required_files() + + +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files_fail_user_map_slack(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["mock_bot", "mock_app", "mock_github"] + mock_get_config.side_effect = [{"owner1": ["repo1"]}, {"github1": ""}] + with pytest.raises(UserMapNotGiven) as exc: + validate_required_files() + assert str(exc.value) == "User github1 does not have a Slack ID assigned." + + +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files_fail_user_map_github(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["mock_bot", "mock_app", "mock_github"] + mock_get_config.side_effect = [{"owner1": ["repo1"]}, {"": "slack1"}] + with pytest.raises(UserMapNotGiven) as exc: + validate_required_files() + assert ( + str(exc.value) + == "Slack member slack1 does not have a GitHub username assigned." + ) -def test_get_maintainer_no_value(): - """This test checks that the defualt user ID is returned if maintainer.txt is empty.""" - with patch("builtins.open", mock_open(read_data="")): - res = get_maintainer() - assert res == "U05RBU0RF4J" # Default Maintainer: Kalibh Halford +@patch("read_data.get_config") +@patch("read_data.get_token") +def test_validate_required_files_fail_user_map(mock_get_token, mock_get_config): + """Test the validate files function""" + mock_get_token.side_effect = ["mock_bot", "mock_app", "mock_github"] + mock_get_config.side_effect = [{"owner1": ["repo1"]}, {}] + with pytest.raises(UserMapNotGiven) as exc: + validate_required_files() + assert str(exc.value) == "config.yml does not contain a user map is empty." diff --git a/cloud-chatops/version.txt b/cloud-chatops/version.txt index 524cb55..38f77a6 100644 --- a/cloud-chatops/version.txt +++ b/cloud-chatops/version.txt @@ -1 +1 @@ -1.1.1 +2.0.1