From 37196f0d59279983c82afa2619d15b0d5e4101a5 Mon Sep 17 00:00:00 2001 From: Tianchen Yang Date: Tue, 27 Oct 2026 23:13:47 +0100 Subject: [PATCH 1/2] run_bot: Fill in the bot name in the --provision error message. When a bot is run by module name with --provision, the error message printed a literal '{}' instead of the bot's name, since the string was never formatted. --- zulip_bots/zulip_bots/run.py | 2 +- zulip_bots/zulip_bots/tests/test_run.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/zulip_bots/zulip_bots/run.py b/zulip_bots/zulip_bots/run.py index 6c8431d7d..d560aa728 100755 --- a/zulip_bots/zulip_bots/run.py +++ b/zulip_bots/zulip_bots/run.py @@ -156,7 +156,7 @@ def main() -> None: bot_name = lib_module.__name__ bot_source = "named module" if args.provision: - print("ERROR: Could not load bot's module for '{}'. Exiting now.") + print(f"ERROR: Could not load bot's module for '{args.bot}'. Exiting now.") sys.exit(1) if lib_module is None: diff --git a/zulip_bots/zulip_bots/tests/test_run.py b/zulip_bots/zulip_bots/tests/test_run.py index ead3f88ab..9b40b7d91 100644 --- a/zulip_bots/zulip_bots/tests/test_run.py +++ b/zulip_bots/zulip_bots/tests/test_run.py @@ -1,5 +1,7 @@ +import io import os import sys +from contextlib import redirect_stdout from pathlib import Path from typing import Optional from unittest import TestCase, mock @@ -127,6 +129,21 @@ def test_run_bot_by_module_name(self, mock_os_path_isfile: mock.Mock) -> None: zulip_bots.run.main() mock_import_module.assert_called_once_with(bot_module_name) + @patch("os.path.isfile", return_value=False) + def test_provision_with_bot_module_name_shows_bot_name( + self, mock_os_path_isfile: mock.Mock + ) -> None: + mock_bot_module = mock.Mock() + mock_bot_module.__name__ = "bot.module.name" + output = io.StringIO() + with patch("sys.argv", ["zulip-run-bot", "bot.module.name", "--provision"]): + with patch("importlib.import_module", return_value=mock_bot_module): + with redirect_stdout(output), self.assertRaises(SystemExit): + zulip_bots.run.main() + + self.assertIn("'bot.module.name'", output.getvalue()) + self.assertNotIn("{}", output.getvalue()) + class TestBotLib(TestCase): def test_extract_query_without_mention(self) -> None: From 50922506955f1d94d215630814d77b254498915d Mon Sep 17 00:00:00 2001 From: Tianchen Yang Date: Tue, 27 Oct 2026 23:29:20 +0100 Subject: [PATCH 2/2] run_bot: Explain where we looked when a bot module can't be found. Previously, when zulip-run-bot couldn't find a bot, it only printed "Could not load bot module", without saying what it had tried. This was confusing for people running their own bots, who usually don't know that a bare name is only looked up among the bundled bots and then as an importable module. The error now names the bot, lists the places that were checked, and suggests passing the path to the bot's .py file. Registry lookups (-r) get their own message. Fixes part of #636. --- zulip_bots/zulip_bots/run.py | 25 ++++++++++++++++++++- zulip_bots/zulip_bots/tests/test_run.py | 29 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/zulip_bots/zulip_bots/run.py b/zulip_bots/zulip_bots/run.py index d560aa728..6a1ec2ec8 100755 --- a/zulip_bots/zulip_bots/run.py +++ b/zulip_bots/zulip_bots/run.py @@ -4,6 +4,7 @@ import logging import os import sys +from pathlib import Path from typing import Optional from zulip_bots import finder @@ -111,6 +112,28 @@ def exit_gracefully_if_bot_config_file_does_not_exist(bot_config_file: Optional[ sys.exit(1) +def print_bot_not_found_error(bot: str, registry: bool) -> None: + if registry: + print( + f'ERROR: Could not find a bot named "{bot}" in the zulip_bots registry.\n' + "Make sure the package that provides it is installed. Exiting now." + ) + return + + bundled_bot_path = Path(finder.current_dir, "bots", bot, bot + ".py") + print( + f"""ERROR: Could not load bot module for "{bot}". Looked for: + - a file at {os.path.abspath(bot)} + - {bundled_bot_path} (one of the bots included with zulip_bots) + - a Python module named "{bot}" that can be imported from PYTHONPATH + +If you wrote this bot yourself, try passing the path to its .py file, e.g.: + zulip-run-bot path/to/{Path(bot).stem}.py --config-file ~/zuliprc + +Exiting now.""" + ) + + def main() -> None: args = parse_args() @@ -160,7 +183,7 @@ def main() -> None: sys.exit(1) if lib_module is None: - print("ERROR: Could not load bot module. Exiting now.") + print_bot_not_found_error(args.bot, args.registry) sys.exit(1) if not args.quiet: diff --git a/zulip_bots/zulip_bots/tests/test_run.py b/zulip_bots/zulip_bots/tests/test_run.py index 9b40b7d91..de02c32ba 100644 --- a/zulip_bots/zulip_bots/tests/test_run.py +++ b/zulip_bots/zulip_bots/tests/test_run.py @@ -144,6 +144,35 @@ def test_provision_with_bot_module_name_shows_bot_name( self.assertIn("'bot.module.name'", output.getvalue()) self.assertNotIn("{}", output.getvalue()) + @patch("os.path.isfile", return_value=False) + def test_bot_not_found_error_suggests_passing_path( + self, mock_os_path_isfile: mock.Mock + ) -> None: + output = io.StringIO() + with patch("sys.argv", ["zulip-run-bot", "mybot", "--config-file", "/path/to/config"]): + with patch("zulip_bots.finder.import_module_by_name", return_value=None): + with redirect_stdout(output), self.assertRaises(SystemExit): + zulip_bots.run.main() + + error = output.getvalue() + self.assertIn('Could not load bot module for "mybot"', error) + self.assertIn(str(Path("bots", "mybot", "mybot.py")), error) + self.assertIn("PYTHONPATH", error) + self.assertIn("zulip-run-bot path/to/mybot.py", error) + + def test_bot_not_found_in_registry_error(self) -> None: + output = io.StringIO() + with patch( + "sys.argv", ["zulip-run-bot", "mybot", "--config-file", "/path/to/config", "-r"] + ): + with patch("zulip_bots.finder.metadata.entry_points", return_value=()): + with redirect_stdout(output), self.assertRaises(SystemExit): + zulip_bots.run.main() + + self.assertIn( + 'Could not find a bot named "mybot" in the zulip_bots registry', output.getvalue() + ) + class TestBotLib(TestCase): def test_extract_query_without_mention(self) -> None: