From 5e2b051e60bf2bd483b4273c578c127f54470b73 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:29:01 +0530 Subject: [PATCH] test(slack): give the MPIM reaction-guard test real teeth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reaction-guard regression test defined a local _should_react lambda and asserted it against itself — a tautology that would stay green even if the production guard at _handle_slack_message reverted to (is_dm or is_mentioned), re-introducing the unmentioned-MPIM reaction spam this PR fixes. Replace it with a shared _reaction_guard helper plus a source-introspection test that pins the production expression: asserts (is_one_to_one_dm or is_mentioned) is present and (is_dm or is_mentioned) is absent. Mutation-checked — reverting the adapter guard now fails the test. Follow-up self-review finding on the salvage of #57339. --- tests/gateway/test_slack_mention.py | 43 ++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/tests/gateway/test_slack_mention.py b/tests/gateway/test_slack_mention.py index 69959ac9489..b3fb0685b3a 100644 --- a/tests/gateway/test_slack_mention.py +++ b/tests/gateway/test_slack_mention.py @@ -5,6 +5,7 @@ Follows the same pattern as test_whatsapp_group_gating.py. """ import sys +import inspect from unittest.mock import MagicMock from gateway.config import Platform, PlatformConfig @@ -324,6 +325,19 @@ def test_dm_always_processed_regardless_of_setting(): # mpim as a DM and thereby exempted it from mention gating + reaction guards). # --------------------------------------------------------------------------- +def _reaction_guard(channel_type, is_mentioned): + """Mirror of the production reaction guard in ``_handle_slack_message``: + + _should_react = (is_one_to_one_dm or is_mentioned) and reactions_enabled + + Only a true 1:1 IM or an explicit @mention earns a reaction; MPIMs and + channels must be @mentioned. ``test_reaction_guard_pinned_to_production_expression`` + pins this to the real source so the two cannot silently drift. + """ + is_one_to_one_dm = channel_type == "im" + return is_one_to_one_dm or is_mentioned + + def test_mpim_unmentioned_strict_mention_ignored(): """MPIM, not mentioned, strict_mention on -> dropped (shared surface).""" adapter = _make_adapter(require_mention=True, strict_mention=True, @@ -369,14 +383,29 @@ def test_one_to_one_im_still_exempt(): def test_mpim_unmentioned_does_not_react(): """Reaction guard: only a 1:1 IM or an @mention earns a reaction. An unmentioned MPIM message must NOT get :eyes:/:white_check_mark: noise.""" - def _should_react(channel_type, is_mentioned): - is_one_to_one_dm = channel_type == "im" - return is_one_to_one_dm or is_mentioned + assert _reaction_guard("mpim", False) is False # the reported spam case + assert _reaction_guard("mpim", True) is True # addressed -> ok + assert _reaction_guard("im", False) is True # 1:1 DM -> ok + assert _reaction_guard("", False) is False # channel, unmentioned - assert _should_react("mpim", False) is False # the reported spam case - assert _should_react("mpim", True) is True # addressed -> ok - assert _should_react("im", False) is True # 1:1 DM -> ok - assert _should_react("", False) is False # channel, unmentioned + +def test_reaction_guard_pinned_to_production_expression(): + """Regression teeth for the reaction guard. + + ``_reaction_guard`` mirrors the production expression at the + ``_should_react = (is_one_to_one_dm or is_mentioned) ...`` site in + ``adapter.py``. This test pins that source line so a revert of the fix + (back to ``is_dm or is_mentioned``, which reacts to unmentioned MPIMs) + fails here instead of silently passing a self-referential lambda. + """ + src = inspect.getsource(SlackAdapter._handle_slack_message) + assert "(is_one_to_one_dm or is_mentioned)" in src, ( + "reaction guard no longer keys off is_one_to_one_dm — an unmentioned " + "MPIM would react again (regression of the group-DM fix)" + ) + assert "(is_dm or is_mentioned)" not in src, ( + "reaction guard reverted to is_dm — MPIMs would react when unmentioned" + ) def test_mentioned_message_always_processed():