From 44b21f4033d2bbc5d011a7417ca13d53e13cea96 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Wed, 8 Jul 2026 16:33:42 -0400 Subject: [PATCH] fix(test): fix aggregated notifier tests + enable CI test suite (INFRA-72) (#39) * fix(test): mock get_settings/get_roster in aggregated tests + enable CI tests (INFRA-72) handle_orders_aggregated now delivers the summary via admin DMs (PR #15), adding get_settings/get_roster calls the aggregated tests never mocked, so they hit live DynamoDB. Mock both and assert the message content on the send_dm path. Set run-tests: true so the suite actually runs in CI. * fix(test): default AWS region in conftest so CI collection doesn't hit NoRegionError (INFRA-72) Handlers build boto3 clients at module load; CI runners have no AWS config, so test collection raised NoRegionError once the suite actually ran. Set a region default before imports (offline client construction; calls are mocked). * fix(test): add repo root to sys.path so CI's bare pytest collects functions.* (INFRA-72) test_aggregate_orders imports functions.aggregate_orders.handler, which needs the repo root on sys.path. python -m pytest injects CWD automatically but CI runs pytest directly, so these 11 tests errored at collection in CI only. --- .github/workflows/ci.yml | 2 ++ tests/conftest.py | 14 ++++++++ tests/test_slack_notifier.py | 64 +++++++++++++++++++++++++++++++----- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ec477f..6cc6efb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,3 +7,5 @@ on: jobs: ci: uses: Sea-Haven-Industries/.github/.github/workflows/ci-python-sam.yaml@fd60e4c9041784f666ac0fdefb9bec3c7fbf5143 # main + with: + run-tests: true diff --git a/tests/conftest.py b/tests/conftest.py index 2cd4096..21173fd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,20 @@ import os import sys +# Several Lambda handlers construct boto3.client(...) at module load. On a CI +# runner with no AWS config this raises NoRegionError during test collection +# (a real region is read from ~/.aws/config locally, masking it). Set a default +# region before any import. Client construction is offline; real calls are mocked. +os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1") +os.environ.setdefault("AWS_REGION", "us-east-1") + +# Add the repo root so `import functions..handler` resolves under a bare +# `pytest` invocation. `python -m pytest` injects the CWD automatically, but CI +# runs `pytest` directly, which does not — hence tests passed locally yet failed +# to collect in CI. +_repo_root = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)) +sys.path.insert(0, _repo_root) + # Add the shared layer source directory so `from shared.db import ...` works # without requiring a real Lambda layer or .aws-sam build. _shared_layer_dir = os.path.join(os.path.dirname(__file__), os.pardir, "src", "shared") diff --git a/tests/test_slack_notifier.py b/tests/test_slack_notifier.py index 306b13a..0b9f778 100644 --- a/tests/test_slack_notifier.py +++ b/tests/test_slack_notifier.py @@ -279,11 +279,15 @@ class TestOrderConfirmed: class TestOrdersAggregated: - @patch("slack_notifier_handler.post_channel_message") + @patch("slack_notifier_handler.send_dm") + @patch("slack_notifier_handler.get_roster") + @patch("slack_notifier_handler.get_settings") @patch("slack_notifier_handler.get_summary") @patch("slack_notifier_handler.current_week", return_value="2026-W19") - def test_orders_aggregated_with_subsidy(self, mock_week, mock_summary, mock_post): - """employee_total < grand_total -> message includes company subsidy line.""" + def test_orders_aggregated_with_subsidy( + self, mock_week, mock_summary, mock_settings, mock_roster, mock_dm + ): + """employee_total < grand_total -> admin DM includes company subsidy line.""" mock_summary.return_value = { "total_employees": 5, "total_meals": 12, @@ -294,13 +298,17 @@ class TestOrdersAggregated: {"meal": "Veggie Bowl", "quantity": Decimal("5")}, ], } + mock_settings.return_value = {"admin_emails": ["admin@x.com"]} + mock_roster.return_value = [ + {"email": "admin@x.com", "name": "Admin A", "slack_user_id": "U100"}, + ] result = handler.handle_orders_aggregated({"event": "orders_aggregated"}) assert result["status"] == "notified" - mock_post.assert_called_once() + mock_dm.assert_called_once() - blocks = mock_post.call_args.args[1] + blocks = mock_dm.call_args.args[2] section_text = blocks[1]["text"]["text"] assert "$150.00" in section_text, "Should show grand total" @@ -308,11 +316,13 @@ class TestOrdersAggregated: assert "$30.00" in section_text, "Should show company subsidy amount" assert "company subsidy" in section_text.lower() - @patch("slack_notifier_handler.post_channel_message") + @patch("slack_notifier_handler.send_dm") + @patch("slack_notifier_handler.get_roster") + @patch("slack_notifier_handler.get_settings") @patch("slack_notifier_handler.get_summary") @patch("slack_notifier_handler.current_week", return_value="2026-W19") def test_orders_aggregated_without_subsidy( - self, mock_week, mock_summary, mock_post + self, mock_week, mock_summary, mock_settings, mock_roster, mock_dm ): """Equal totals -> no subsidy line.""" mock_summary.return_value = { @@ -324,11 +334,15 @@ class TestOrdersAggregated: {"meal": "Pasta Primavera", "quantity": Decimal("6")}, ], } + mock_settings.return_value = {"admin_emails": ["admin@x.com"]} + mock_roster.return_value = [ + {"email": "admin@x.com", "name": "Admin A", "slack_user_id": "U100"}, + ] result = handler.handle_orders_aggregated({"event": "orders_aggregated"}) assert result["status"] == "notified" - blocks = mock_post.call_args.args[1] + blocks = mock_dm.call_args.args[2] section_text = blocks[1]["text"]["text"] assert "company subsidy" not in section_text.lower(), ( @@ -347,6 +361,40 @@ class TestOrdersAggregated: assert result["status"] == "no_summary" mock_post.assert_not_called() + @patch("slack_notifier_handler.send_dm") + @patch("slack_notifier_handler.get_roster") + @patch("slack_notifier_handler.get_settings") + @patch("slack_notifier_handler.post_channel_message") + @patch("slack_notifier_handler.get_summary") + @patch("slack_notifier_handler.current_week", return_value="2026-W19") + def test_orders_aggregated_dms_admins( + self, mock_week, mock_summary, mock_post, mock_settings, mock_roster, mock_dm + ): + """Admins with a Slack ID get DM'd the summary; others are skipped.""" + mock_summary.return_value = { + "total_employees": 2, + "total_meals": 4, + "grand_total": Decimal("60.00"), + "employee_total": Decimal("60.00"), + "meals": [ + {"meal": "Grilled Chicken", "quantity": Decimal("4")}, + ], + } + mock_settings.return_value = { + "admin_emails": ["Admin@x.com", "noslack@x.com", "missing@x.com"], + } + mock_roster.return_value = [ + {"email": "admin@x.com", "name": "Admin A", "slack_user_id": "U100"}, + {"email": "noslack@x.com", "name": "No Slack"}, # no slack_user_id + ] + + result = handler.handle_orders_aggregated({"event": "orders_aggregated"}) + + assert result["status"] == "notified" + assert result["admin_dm_count"] == 1, "Only the admin with a Slack ID is DM'd" + mock_dm.assert_called_once() + assert mock_dm.call_args.args[0] == "U100" + # ──────────────────────────────────────────────────────────────────────────── # Escaping (Low)