tests: Exercise Merels adapter move path; add minimal test helpers.

Merels previously had adapter-focused tests for help/start/join, but
the move path through GameAdapter was untested. This adds a small
in-file test helper to drive the adapter and a test that starts a
2-player game, sends a move, counts model.make_move calls, and asserts
that the bot replies. This covers the 'test lib for game_handler'
FIXME.

I did not add 'computer move' tests, because the Merels bot declares
supports_computer = False; there is no single-player/computer flow
to exercise. I left a comment in above TestMerelsAdapter that
clarifies this.

No production changes; tests only. Passes local pytest, mypy, and
lint.

Fixes #433.

Author: sulahmed20

zulip_bots/zulip_bots/bots/merels/test_merels.py

@@ -11,7 +11,7 @@ class TestMerelsBot(BotTestCase, DefaultTests):
     bot_name = "merels"
 
     def test_no_command(self) -> None:
-        # Sanity: out-of-game message for random content.
+        # Out-of-game message for arbitrary input.
         message = dict(
             content="magic", type="stream", sender_email="boo@email.com", sender_full_name="boo"
         )
@@ -21,25 +21,60 @@ def test_no_command(self) -> None:
         )
 
     def test_parse_board_identity_empty_board(self) -> None:
-        # parse_board is identity for Merels; verify with the canonical empty board.
+        # Merels parse_board is identity; verify with the canonical empty board.
         bot, _ = self._get_handlers()
         self.assertEqual(bot.game_message_handler.parse_board(EMPTY_BOARD), EMPTY_BOARD)
 
 
-class TestMerelsAdapter(BotTestCase, DefaultTests):
-    """
-    Adapter-focused tests mirroring connect_four, kept in this file to
-    keep Merels tests cohesive. Assert on stable fragments to avoid brittle
-    exact-string matches.
-    """
+class GameAdapterTestLib:
+    """Small helpers for driving GameAdapter-based bots in tests."""
+
+    def send(
+        self,
+        bot,
+        bot_handler,
+        content: str,
+        *,
+        user: str = "foo@example.com",
+        user_name: str = "foo",
+    ) -> None:
+        bot.handle_message(
+            self.make_request_message(content, user=user, user_name=user_name),
+            bot_handler,
+        )
+
+    def replies(self, bot_handler):
+        # Return the bot message 'content' fields from the transcript.
+        return [m["content"] for (_method, m) in bot_handler.transcript]
+
+    def send_and_collect(
+        self,
+        bot,
+        bot_handler,
+        content: str,
+        *,
+        user: str = "foo@example.com",
+        user_name: str = "foo",
+    ):
+        bot_handler.reset_transcript()
+        self.send(bot, bot_handler, content, user=user, user_name=user_name)
+        return self.replies(bot_handler)
+
+
+# Note: Merels has no vs-computer mode (in merels.py, supports_computer=False).
+# If computer mode is added in the future, add adapter-level tests here.
+
+
+class TestMerelsAdapter(BotTestCase, DefaultTests, GameAdapterTestLib):
+    """Adapter-focused tests (mirrors connect_four); use stable fragment assertions."""
 
     bot_name = "merels"
 
     @override
     def make_request_message(
         self, content: str, user: str = "foo@example.com", user_name: str = "foo"
     ) -> Dict[str, str]:
-        # Provide stream metadata; GameAdapter reads message["type"], topic, etc.
+        # Provide stream metadata consumed by GameAdapter.
         return {
             "sender_email": user,
             "sender_full_name": user_name,
@@ -59,13 +94,12 @@ def test_help_is_merels_help(self) -> None:
         self.assertTrue(responses, "No bot response to 'help'")
         help_text = responses[0]["content"]
 
-        # Stable fragments; resilient to copy tweaks.
+        # Assert on stable fragments to avoid brittle exact matches.
         self.assertIn("Merels Bot Help", help_text)
         self.assertIn("start game", help_text)
         self.assertIn("play game", help_text)
         self.assertIn("quit", help_text)
         self.assertIn("rules", help_text)
-        # Present today; OK if dropped in future wording changes.
         self.assertIn("leaderboard", help_text)
         self.assertIn("cancel game", help_text)
 
@@ -104,12 +138,12 @@ def test_join_starts_game_emits_start_message(self) -> None:
     def test_message_handler_helpers(self) -> None:
         bot, _ = self._get_handlers()
 
-        # parse_board returns the given board representation.
+        # Identity parse_board.
         self.assertEqual(
             bot.game_message_handler.parse_board("sample_board_repr"), "sample_board_repr"
         )
 
-        # Token color is one of the two known emoji.
+        # Token color in allowed set.
         self.assertIn(
             bot.game_message_handler.get_player_color(0),
             (":o_button:", ":cross_mark_button:"),
@@ -124,3 +158,46 @@ def test_message_handler_helpers(self) -> None:
             bot.game_message_handler.alert_move_message("foo", "move 1,1"),
             "foo :move 1,1",
         )
+
+    def test_move_after_join_invokes_make_move_and_replies(self) -> None:
+        """
+        Start a two-player game, send a move via the adapter, count make_move calls,
+        and assert we get a reply. Try both users to avoid turn assumptions.
+        """
+        import types
+
+        bot, bot_handler = self._get_handlers()
+
+        # Start 2P game.
+        _ = self.send_and_collect(
+            bot, bot_handler, "start game", user="foo@example.com", user_name="foo"
+        )
+        _ = self.send_and_collect(bot, bot_handler, "join", user="bar@example.com", user_name="bar")
+
+        # Count model.make_move invocations.
+        self.assertTrue(hasattr(bot.model, "make_move"), "Merels model has no make_move method")
+        original = bot.model.make_move
+        calls = {"n": 0}
+
+        def _wrapped_make_move(*args, **kwargs):
+            calls["n"] += 1
+            return original(*args, **kwargs)
+
+        bot.model.make_move = types.MethodType(_wrapped_make_move, bot.model)  # type: ignore[attr-defined]
+        try:
+            contents_foo = self.send_and_collect(
+                bot, bot_handler, "move 1,1", user="foo@example.com", user_name="foo"
+            )
+            if calls["n"] == 0:
+                contents_bar = self.send_and_collect(
+                    bot, bot_handler, "move 1,1", user="bar@example.com", user_name="bar"
+                )
+            else:
+                contents_bar = []
+
+            self.assertGreaterEqual(calls["n"], 1, "make_move was not called for a move command")
+            self.assertTrue(
+                contents_foo or contents_bar, "No bot reply after sending a move command"
+            )
+        finally:
+            bot.model.make_move = original  # type: ignore[assignment]