diff --git a/cloud-chatops/.coveragerc b/cloud-chatops/.coveragerc index b2249df..160c5d2 100644 --- a/cloud-chatops/.coveragerc +++ b/cloud-chatops/.coveragerc @@ -1,3 +1,5 @@ [run] omit = - /usr/lib/** \ No newline at end of file + # Omitting this directory as sometimes it's included in the coverage. + # This is likely due to messy Python Paths and environments + /usr/lib/** diff --git a/cloud-chatops/src/features/base_feature.py b/cloud-chatops/src/features/base_feature.py index 51d6970..46071e3 100644 --- a/cloud-chatops/src/features/base_feature.py +++ b/cloud-chatops/src/features/base_feature.py @@ -3,6 +3,7 @@ All features should inherit this class to reduce code duplication. """ +from abc import ABC from typing import Dict, List from datetime import datetime, timedelta from dataclasses import replace @@ -21,8 +22,8 @@ DEFAULT_REPO_OWNER = "stfc" # Our repos are owned by "stfc" -class BaseFeature: - """This base class provides universal methods for features.""" +class BaseFeature(ABC): + """This base abstract class provides universal methods for features.""" # pylint: disable=R0903 # This is a base feature and not intended to be used on its own @@ -94,7 +95,7 @@ def _send_thread_react(self, pr: PrData, channel: str, thread_ts: str) -> None: if pr.old: react_with.append("alarm_clock") if pr.draft: - react_with.append("scroll") + react_with.append("building_construction") for react in react_with: react_response = self.client.reactions_add( channel=channel, name=react, timestamp=thread_ts diff --git a/cloud-chatops/src/read_data.py b/cloud-chatops/src/read_data.py index 7e97ff5..3b71000 100644 --- a/cloud-chatops/src/read_data.py +++ b/cloud-chatops/src/read_data.py @@ -10,6 +10,9 @@ TokensNotGiven, ) +# 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 = "/usr/src/app/dev_cloud_chatops_secrets/" try: if sys.argv[1] == "local": diff --git a/cloud-chatops/tests/test_base_feature.py b/cloud-chatops/tests/test_base_feature.py index adb0427..9f69a79 100644 --- a/cloud-chatops/tests/test_base_feature.py +++ b/cloud-chatops/tests/test_base_feature.py @@ -1,7 +1,4 @@ """Tests for features.base_feature.BaseFeature""" - -# pylint: disable=W0212 -# Disabling this as we need to access protected methods to test them from unittest.mock import NonCallableMock, patch, MagicMock import pytest from slack_sdk.errors import SlackApiError @@ -15,11 +12,11 @@ @patch("features.base_feature.get_repos") @patch("features.base_feature.get_user_map") def instance_fixture( - mock_get_user_map, - mock_get_repos, - mock_get_token, - mock_get_github_prs, - mock_web_client, + 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"} @@ -27,18 +24,50 @@ def instance_fixture( mock_get_token.return_value = "mock_slack_token" mock_get_github_prs.run.return_value = [] mock_web_client.return_value = NonCallableMock() - return BaseFeature() + + class BaseFeatureWrapper(BaseFeature): + """A wrapper class to make the private methods accessible""" + + def github_to_slack_username(self, user: str): + """Returns the private method""" + return self._github_to_slack_username(user) + + def slack_to_human_username(self, slack_member_id): + """Returns the private method""""" + return self._slack_to_human_username(slack_member_id) + + def post_reminder_message(self): + """Returns the private method""""" + return self._post_reminder_message() + + def send_no_prs_found(self, thread_ts): + """Returns the private method""""" + return self._send_no_prs_found(thread_ts) + + def send_thread(self, pr_data, thread_ts): + """Returns the private method""""" + return self._send_thread(pr_data, thread_ts) + + def send_thread_react(self, pr, channel, thread_ts): + """Returns the private method""""" + return self._send_thread_react(pr, channel, thread_ts) + + def format_prs(self, prs): + """Returns the private method""""" + return self._format_prs(prs) + + return BaseFeatureWrapper() def test_github_to_slack_username_valid(instance): """Test the github to slack name translation works""" - res = instance._github_to_slack_username("mock_github") + res = instance.github_to_slack_username("mock_github") assert res == "mock_slack" def test_github_to_slack_username_invalid(instance): """Test the github to slack name translation works""" - res = instance._github_to_slack_username("mock_github_not_found") + res = instance.github_to_slack_username("mock_github_not_found") assert res == "mock_github_not_found" @@ -47,7 +76,7 @@ def test_slack_to_human_username_valid(instance): instance.client.users_profile_get.return_value = { "profile": {"real_name": "mock_name"} } - res = instance._slack_to_human_username("mock_member_id") + res = instance.slack_to_human_username("mock_member_id") instance.client.users_profile_get.assert_called_once_with(user="mock_member_id") assert res == "mock_name" @@ -55,7 +84,7 @@ def test_slack_to_human_username_valid(instance): def test_slack_to_human_username_invalid(instance): """Test the slack member id to real name works""" instance.client.users_profile_get.side_effect = SlackApiError("", "") - res = instance._slack_to_human_username("mock_member_id") + res = instance.slack_to_human_username("mock_member_id") assert res == "mock_member_id" @@ -65,7 +94,7 @@ def test_post_reminder_message(instance): return_obj.data = {"ts": 100} return_obj.__getitem__.side_effect = [True] instance.client.chat_postMessage.return_value = return_obj - res = instance._post_reminder_message() + res = instance.post_reminder_message() instance.client.chat_postMessage.assert_called_once_with( channel=instance.channel, text="Here are the outstanding PRs as of today:", @@ -77,13 +106,13 @@ def test_post_reminder_message_fails(instance): """Test a message is not posted""" instance.client.chat_postMessage.return_value = {"ok": False} with pytest.raises(AssertionError): - instance._post_reminder_message() + instance.post_reminder_message() def test_send_no_prs_found(instance): """Test a message is posted""" instance.client.chat_postMessage.return_value = {"ok": True} - instance._send_no_prs_found(100) + instance.send_no_prs_found(100) instance.client.chat_postMessage.assert_called_once_with( channel=instance.channel, thread_ts=100, @@ -96,7 +125,7 @@ def test_send_no_prs_found_fails(instance): """Test a message is not posted""" instance.client.chat_postMessage.return_value = {"ok": False} with pytest.raises(AssertionError): - instance._send_no_prs_found(100) + instance.send_no_prs_found(100) @patch("features.base_feature.PRMessageBuilder") @@ -105,7 +134,7 @@ def test_send_thread(mock_pr_message_builder, instance): mock_pr_data = NonCallableMock() mock_pr_message_builder.return_value.make_message.return_value = "mock_message" instance.client.chat_postMessage.return_value = {"ok": True} - res = instance._send_thread(mock_pr_data, "100") + res = instance.send_thread(mock_pr_data, "100") mock_pr_message_builder.return_value.make_message.assert_called_once_with( mock_pr_data ) @@ -123,7 +152,7 @@ def test_send_thread_fails(_, instance): """Test a message is not sent""" instance.client.chat_postMessage.return_value = {"ok": False} with pytest.raises(AssertionError): - instance._send_thread(NonCallableMock(), "100") + instance.send_thread(NonCallableMock(), "100") def test_send_thread_react_no_reactions(instance): @@ -131,7 +160,7 @@ def test_send_thread_react_no_reactions(instance): mock_pr_data = NonCallableMock() mock_pr_data.old = False mock_pr_data.draft = False - instance._send_thread_react(mock_pr_data, "mock_channel", "100") + instance.send_thread_react(mock_pr_data, "mock_channel", "100") instance.client.reactions_add.assert_not_called() @@ -142,7 +171,7 @@ def test_send_thread_react_fails_to_add(instance): mock_pr_data.draft = True instance.client.reactions_add.side_effect = [{"ok": True}, {"ok": False}] with pytest.raises(AssertionError): - instance._send_thread_react(mock_pr_data, "mock_channel", "100") + instance.send_thread_react(mock_pr_data, "mock_channel", "100") def test_send_thread_react_with_reactions(instance): @@ -151,10 +180,10 @@ def test_send_thread_react_with_reactions(instance): mock_pr_data.old = True mock_pr_data.draft = True instance.client.reactions_add.return_value = {"ok": True} - instance._send_thread_react(mock_pr_data, "mock_channel", "100") + instance.send_thread_react(mock_pr_data, "mock_channel", "100") instance.client.reactions_add.assert_any_call( channel="mock_channel", name="alarm_clock", timestamp="100" ) instance.client.reactions_add.assert_any_call( - channel="mock_channel", name="scroll", timestamp="100" + channel="mock_channel", name="building_construction", timestamp="100" )