From 12592aabeb8cbe079d182f4c5d4cf78d9791e989 Mon Sep 17 00:00:00 2001 From: Beenyaa Date: Fri, 6 Feb 2026 16:06:20 +0000 Subject: [PATCH 1/3] feat: change logging + tests --- .../notifyS3UploadFunction/tests/conftest.py | 1 - .../slackBotFunction/app/services/bedrock.py | 1 - .../slackBotFunction/app/services/slack.py | 6 +- .../app/slack/slack_events.py | 9 +- .../tests/test_privacy_changes.py | 93 +++++++++++++++++++ scripts/run_regression_tests.py | 1 + 6 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 packages/slackBotFunction/tests/test_privacy_changes.py diff --git a/packages/notifyS3UploadFunction/tests/conftest.py b/packages/notifyS3UploadFunction/tests/conftest.py index fcc73440d..d81fc8142 100644 --- a/packages/notifyS3UploadFunction/tests/conftest.py +++ b/packages/notifyS3UploadFunction/tests/conftest.py @@ -4,7 +4,6 @@ from unittest.mock import MagicMock, Mock, patch import os - TEST_BOT_TOKEN = "test-bot-token" diff --git a/packages/slackBotFunction/app/services/bedrock.py b/packages/slackBotFunction/app/services/bedrock.py index 44d020196..8bb06f0b3 100644 --- a/packages/slackBotFunction/app/services/bedrock.py +++ b/packages/slackBotFunction/app/services/bedrock.py @@ -8,7 +8,6 @@ from app.core.config import get_retrieve_generate_config, get_logger from app.services.prompt_loader import load_prompt - logger = get_logger() diff --git a/packages/slackBotFunction/app/services/slack.py b/packages/slackBotFunction/app/services/slack.py index 66fe280e0..288718b86 100644 --- a/packages/slackBotFunction/app/services/slack.py +++ b/packages/slackBotFunction/app/services/slack.py @@ -3,7 +3,6 @@ from slack_sdk import WebClient from app.core.config import bot_messages, get_logger - logger = get_logger() @@ -12,7 +11,10 @@ def get_friendly_channel_name(channel_id: str, client: WebClient) -> str: try: conversations_info_response = client.conversations_info(channel=channel_id) if conversations_info_response["ok"]: - friendly_channel_name = conversations_info_response["channel"]["name"] + channel_info = conversations_info_response["channel"] + if channel_info.get("is_im"): + return "Direct Message" + friendly_channel_name = channel_info.get("name", channel_id) else: logger.warning( "There was a problem getting the friendly channel name", diff --git a/packages/slackBotFunction/app/slack/slack_events.py b/packages/slackBotFunction/app/slack/slack_events.py index 726968cc6..5453d6867 100644 --- a/packages/slackBotFunction/app/slack/slack_events.py +++ b/packages/slackBotFunction/app/slack/slack_events.py @@ -40,7 +40,6 @@ from app.services.ai_processor import process_ai_query - logger = get_logger() processing_error_message = "Error processing message" @@ -506,7 +505,7 @@ def process_slack_message(event: Dict[str, Any], event_id: str, client: WebClien user_query = re.sub(r"<@[UW][A-Z0-9]+(\|[^>]+)?>", "", event["text"]).strip() logger.info( - f"Processing message from user {user_id}", + "Processing message", extra={"user_query": user_query, "conversation_key": conversation_key, "event_id": event_id}, ) @@ -570,7 +569,11 @@ def log_query_stats(user_query: str, event: Dict[str, Any], channel: str, client end_time = time.time() duration = end_time - start_time is_direct_message = event.get("channel_type") == constants.CHANNEL_TYPE_IM - friendly_channel_name = get_friendly_channel_name(channel_id=channel, client=client) + if is_direct_message: + friendly_channel_name = "Direct Message" + else: + friendly_channel_name = get_friendly_channel_name(channel_id=channel, client=client) + reporting_info = { "query_length": query_length, "start_time": start_time, diff --git a/packages/slackBotFunction/tests/test_privacy_changes.py b/packages/slackBotFunction/tests/test_privacy_changes.py new file mode 100644 index 000000000..5f97f7ea0 --- /dev/null +++ b/packages/slackBotFunction/tests/test_privacy_changes.py @@ -0,0 +1,93 @@ +from unittest.mock import Mock, patch +from app.services.slack import get_friendly_channel_name +from app.slack.slack_events import process_slack_message, log_query_stats +import sys + +# Remove modules from cache to ensure fresh import with mocks if needed +for module in list(sys.modules.keys()): + if module.startswith("app"): + del sys.modules[module] + + +def test_get_friendly_channel_name_returns_direct_message_for_im(): + mock_client = Mock() + mock_client.conversations_info.return_value = {"ok": True, "channel": {"is_im": True, "id": "D12345"}} + + result = get_friendly_channel_name("D12345", mock_client) + assert result == "Direct Message" + + +def test_get_friendly_channel_name_returns_name_for_public_channel(): + mock_client = Mock() + mock_client.conversations_info.return_value = { + "ok": True, + "channel": {"is_im": False, "name": "general", "id": "C12345"}, + } + + result = get_friendly_channel_name("C12345", mock_client) + assert result == "general" + + +@patch("app.slack.slack_events.logger") +def test_log_query_stats_masks_dm_channel(mock_logger, mock_env, mock_get_parameter): + mock_client = Mock() + # verify we don't call client.conversations_info because we handle it explicitly + + event = {"event_ts": "1234567890.123", "channel_type": "im"} + + log_query_stats(user_query="test", event=event, channel="D123", client=mock_client, thread_ts="123") + + # Check if info was called + assert mock_logger.info.called + + # Inspect arguments + args, kwargs = mock_logger.info.call_args + reporting_info = kwargs["extra"]["reporting_info"] + assert reporting_info["channel"] == "Direct Message" + assert reporting_info["is_direct_message"] is True + # Ensure no API call was made to fetch channel name (optimization check) + mock_client.conversations_info.assert_not_called() + + +@patch("app.slack.slack_events.logger") +@patch("app.slack.slack_events.conversation_key_and_root") +@patch("app.slack.slack_events.get_conversation_session_data") +@patch("app.slack.slack_events.get_friendly_channel_name") +@patch("app.services.ai_processor.process_ai_query") +def test_process_slack_message_log_privacy( + mock_process_ai, mock_get_friendly, mock_get_session, mock_key_root, mock_logger, mock_env, mock_get_parameter +): + mock_key_root.return_value = ("key", "ts") + mock_client = Mock() + mock_client.chat_postMessage.return_value = {"ts": "123"} + mock_get_session.return_value = {} + mock_process_ai.return_value = {"kb_response": {}, "text": "response", "session_id": "sid"} + + event = { + "user": "U12345", + "channel": "C123", + "text": "hello", + "ts": "123", + "event_ts": "123", + "channel_type": "channel", + } + + process_slack_message(event, "evt1", mock_client) + + # Verify the "Processing message" log call + # It should NOT contain the user ID "U12345" in the message string + + # Find the call with "Processing message" + processing_call = None + for call in mock_logger.info.call_args_list: + args, _ = call + if args and "Processing message" in args[0]: + processing_call = call + break + + assert processing_call is not None + # The message should be exactly "Processing message" (or at least not contain "from user U12345") + # call args is a tuple (args, kwargs) + args, _ = processing_call + assert "from user" not in args[0] + assert "U12345" not in args[0] diff --git a/scripts/run_regression_tests.py b/scripts/run_regression_tests.py index 772c6a5f9..be906eb57 100644 --- a/scripts/run_regression_tests.py +++ b/scripts/run_regression_tests.py @@ -4,6 +4,7 @@ Script to generate user defined unique ID which can be used to check the status of the regression test run to be reported to the CI. """ + import argparse from datetime import datetime, timedelta, timezone import random From 13a0e307c1dc023026beb36140083e8442c31c48 Mon Sep 17 00:00:00 2001 From: Bence Gadanyi Date: Tue, 17 Feb 2026 11:32:59 +0000 Subject: [PATCH 2/3] fix: test setup --- .../tests/test_privacy_changes.py | 127 +++++++++--------- poetry.lock | 2 +- 2 files changed, 63 insertions(+), 66 deletions(-) diff --git a/packages/slackBotFunction/tests/test_privacy_changes.py b/packages/slackBotFunction/tests/test_privacy_changes.py index 5f97f7ea0..953c23792 100644 --- a/packages/slackBotFunction/tests/test_privacy_changes.py +++ b/packages/slackBotFunction/tests/test_privacy_changes.py @@ -1,12 +1,6 @@ +import sys from unittest.mock import Mock, patch from app.services.slack import get_friendly_channel_name -from app.slack.slack_events import process_slack_message, log_query_stats -import sys - -# Remove modules from cache to ensure fresh import with mocks if needed -for module in list(sys.modules.keys()): - if module.startswith("app"): - del sys.modules[module] def test_get_friendly_channel_name_returns_direct_message_for_im(): @@ -28,66 +22,69 @@ def test_get_friendly_channel_name_returns_name_for_public_channel(): assert result == "general" -@patch("app.slack.slack_events.logger") -def test_log_query_stats_masks_dm_channel(mock_logger, mock_env, mock_get_parameter): +def test_log_query_stats_masks_dm_channel(mock_env, mock_get_parameter): + # reload module to ensure clean state and correct patching + if "app.slack.slack_events" in sys.modules: + del sys.modules["app.slack.slack_events"] + + from app.slack.slack_events import log_query_stats + import app.slack.slack_events + mock_client = Mock() - # verify we don't call client.conversations_info because we handle it explicitly event = {"event_ts": "1234567890.123", "channel_type": "im"} - log_query_stats(user_query="test", event=event, channel="D123", client=mock_client, thread_ts="123") - - # Check if info was called - assert mock_logger.info.called - - # Inspect arguments - args, kwargs = mock_logger.info.call_args - reporting_info = kwargs["extra"]["reporting_info"] - assert reporting_info["channel"] == "Direct Message" - assert reporting_info["is_direct_message"] is True - # Ensure no API call was made to fetch channel name (optimization check) - mock_client.conversations_info.assert_not_called() - - -@patch("app.slack.slack_events.logger") -@patch("app.slack.slack_events.conversation_key_and_root") -@patch("app.slack.slack_events.get_conversation_session_data") -@patch("app.slack.slack_events.get_friendly_channel_name") -@patch("app.services.ai_processor.process_ai_query") -def test_process_slack_message_log_privacy( - mock_process_ai, mock_get_friendly, mock_get_session, mock_key_root, mock_logger, mock_env, mock_get_parameter -): - mock_key_root.return_value = ("key", "ts") - mock_client = Mock() - mock_client.chat_postMessage.return_value = {"ts": "123"} - mock_get_session.return_value = {} - mock_process_ai.return_value = {"kb_response": {}, "text": "response", "session_id": "sid"} - - event = { - "user": "U12345", - "channel": "C123", - "text": "hello", - "ts": "123", - "event_ts": "123", - "channel_type": "channel", - } + with patch.object(app.slack.slack_events, "logger") as mock_logger: + log_query_stats(user_query="test", event=event, channel="D123", client=mock_client, thread_ts="123") + + assert mock_logger.info.called + + args, kwargs = mock_logger.info.call_args + reporting_info = kwargs["extra"]["reporting_info"] + assert reporting_info["channel"] == "Direct Message" + assert reporting_info["is_direct_message"] is True + + mock_client.conversations_info.assert_not_called() + + +def test_process_slack_message_log_privacy(mock_env, mock_get_parameter): + if "app.slack.slack_events" in sys.modules: + del sys.modules["app.slack.slack_events"] + + from app.slack.slack_events import process_slack_message + import app.slack.slack_events + + with patch.object(app.slack.slack_events, "logger") as mock_logger, patch.object( + app.slack.slack_events, "conversation_key_and_root", return_value=("key", "ts") + ), patch.object(app.slack.slack_events, "get_conversation_session_data", return_value={}), patch.object( + app.slack.slack_events, "get_friendly_channel_name" + ), patch.object( + app.slack.slack_events, "process_ai_query" + ) as mock_process_ai: + + mock_client = Mock() + mock_client.chat_postMessage.return_value = {"ts": "123"} + mock_process_ai.return_value = {"kb_response": {}, "text": "response", "session_id": "sid"} + + event = { + "user": "U12345", + "channel": "C123", + "text": "hello", + "ts": "123", + "event_ts": "123", + "channel_type": "channel", + } + + process_slack_message(event, "evt1", mock_client) + + processing_call = None + for call in mock_logger.info.call_args_list: + args, _ = call + if args and "Processing message" in args[0]: + processing_call = call + break - process_slack_message(event, "evt1", mock_client) - - # Verify the "Processing message" log call - # It should NOT contain the user ID "U12345" in the message string - - # Find the call with "Processing message" - processing_call = None - for call in mock_logger.info.call_args_list: - args, _ = call - if args and "Processing message" in args[0]: - processing_call = call - break - - assert processing_call is not None - # The message should be exactly "Processing message" (or at least not contain "from user U12345") - # call args is a tuple (args, kwargs) - args, _ = processing_call - assert "from user" not in args[0] - assert "U12345" not in args[0] + assert processing_call is not None + args, _ = processing_call + assert "from user" not in args[0] + assert "U12345" not in args[0] diff --git a/poetry.lock b/poetry.lock index 208933745..850f2d736 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1091,7 +1091,7 @@ files = [ {file = "colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6"}, {file = "colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44"}, ] -markers = {dev = "platform_system == \"Windows\" or sys_platform == \"win32\"", preprocessingfunction = "platform_system == \"Windows\""} +markers = {dev = "sys_platform == \"win32\" or platform_system == \"Windows\"", preprocessingfunction = "platform_system == \"Windows\""} [[package]] name = "coverage" From ec9bc73212d571bfb5d864163b13557abef2b12d Mon Sep 17 00:00:00 2001 From: Bence Gadanyi Date: Tue, 17 Feb 2026 11:49:51 +0000 Subject: [PATCH 3/3] fix: test setup --- packages/slackBotFunction/tests/test_privacy_changes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/slackBotFunction/tests/test_privacy_changes.py b/packages/slackBotFunction/tests/test_privacy_changes.py index 953c23792..56639acde 100644 --- a/packages/slackBotFunction/tests/test_privacy_changes.py +++ b/packages/slackBotFunction/tests/test_privacy_changes.py @@ -39,7 +39,7 @@ def test_log_query_stats_masks_dm_channel(mock_env, mock_get_parameter): assert mock_logger.info.called - args, kwargs = mock_logger.info.call_args + _, kwargs = mock_logger.info.call_args reporting_info = kwargs["extra"]["reporting_info"] assert reporting_info["channel"] == "Direct Message" assert reporting_info["is_direct_message"] is True