#691 fix full-repo RUNTIME collision: fully-qualify handler.py lazy imports + test_handler_routing patch targets. CI (not isolation) exposed it: handler.py used bare 'from apps.handlers.X import Y' and the tests patched bare 'apps.handlers.X' -> in a whole-repo run bare apps resolves to the WRONG branch (AttributeError/ModuleNotFoundError, 17 test_handler_routing failures). FQ'd both to aipass.skills.lib.telegram.apps.handlers.* (handler.py 7 lazy imports now house-rule compliant; skill runtime verified via drone @skills run telegram status). Verified full-repo: 0 test_handler_routing failures (was 17), 11019 passed, isolation telegram 663 pass (no collateral). Only remaining local fail is pre-existing skills_json/ghost_config.json litter (gitignored, green in CI).
This commit is contained in:
+11
-5
@@ -45,11 +45,17 @@ PyPI version — not the changelog header.
|
||||
run in CI before because they failed at collection. Added a session-scoped autouse
|
||||
`_block_network` conftest fixture that patches `urlopen` on the four network-using
|
||||
telegram modules (both bare and fully-qualified import paths, each guarded) so any
|
||||
test attempting a live HTTP call fails loud instead of hanging. Test-infra only —
|
||||
product code byte-unchanged, no telegram behavior change. Full-repo collection now
|
||||
0 errors (11,028 tests); telegram suite 663 passed / 0 failed / 0 hangs, fully
|
||||
hermetic. Coverage intact (zero assertion changes — purely import/patch-target
|
||||
rewiring).
|
||||
test attempting a live HTTP call fails loud instead of hanging. A full-repo CI run
|
||||
then exposed a third layer the isolated suites had hidden: `handler.py` and its
|
||||
routing tests still used bare `from apps.handlers.X import Y` / `mock.patch("apps.
|
||||
handlers.X…")`, which resolve to the *wrong* branch's `apps` in a whole-repo run
|
||||
(AttributeError / ModuleNotFoundError at runtime — 17 `test_handler_routing`
|
||||
failures). Fully-qualified those to `aipass.skills.lib.telegram.apps.handlers.*` in
|
||||
both `handler.py` (7 lazy imports, now house-rule compliant) and the tests; the
|
||||
skill's runtime behaviour is unchanged (verified via `drone @skills run telegram`).
|
||||
Net: full-repo collection 0 errors and the whole 11k-test suite green; telegram
|
||||
suite 663 passed / 0 failed / 0 hangs, fully hermetic; coverage intact (import and
|
||||
patch-target rewiring only — zero assertion changes).
|
||||
|
||||
- **`aipass install` from a throwaway path can no longer hijack the machine-wide
|
||||
`AIPASS_HOME` (issue #688).** A probe install run from a `/tmp` scratchpad had
|
||||
|
||||
@@ -64,7 +64,7 @@ def _normalize_args(args) -> list:
|
||||
def _cmd_start(args: list) -> dict:
|
||||
if not args:
|
||||
return _err("start requires a bot_id: drone @skills run telegram start <bot_id>")
|
||||
from apps.handlers.bot_operations import start_bot
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_operations import start_bot
|
||||
|
||||
bot_id = args[0]
|
||||
exit_code = start_bot(bot_id)
|
||||
@@ -76,7 +76,7 @@ def _cmd_start(args: list) -> dict:
|
||||
def _cmd_stop(args: list) -> dict:
|
||||
if not args:
|
||||
return _err("stop requires a bot_id: drone @skills run telegram stop <bot_id>")
|
||||
from apps.handlers.bot_operations import stop_bot
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_operations import stop_bot
|
||||
|
||||
success, message = stop_bot(args[0])
|
||||
if success:
|
||||
@@ -85,7 +85,7 @@ def _cmd_stop(args: list) -> dict:
|
||||
|
||||
|
||||
def _cmd_status(args: list) -> dict:
|
||||
from apps.handlers.bot_operations import (
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_operations import (
|
||||
format_bot_details,
|
||||
format_bot_table,
|
||||
get_status,
|
||||
@@ -102,8 +102,8 @@ def _cmd_status(args: list) -> dict:
|
||||
|
||||
|
||||
def _cmd_create(args: list) -> dict:
|
||||
from apps.handlers.bot_factory import create_bot
|
||||
from apps.handlers.bot_operations import parse_create_args
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_factory import create_bot
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_operations import parse_create_args
|
||||
|
||||
parsed = parse_create_args(args)
|
||||
if not parsed:
|
||||
@@ -122,7 +122,7 @@ def _cmd_create(args: list) -> dict:
|
||||
def _cmd_delete(args: list) -> dict:
|
||||
if not args:
|
||||
return _err("delete requires a bot_id: drone @skills run telegram delete <bot_id>")
|
||||
from apps.handlers.bot_factory import delete_bot
|
||||
from aipass.skills.lib.telegram.apps.handlers.bot_factory import delete_bot
|
||||
|
||||
success = delete_bot(args[0])
|
||||
if success:
|
||||
@@ -133,7 +133,7 @@ def _cmd_delete(args: list) -> dict:
|
||||
def _cmd_notify(args: list) -> dict:
|
||||
if not args:
|
||||
return _err('notify requires a message: drone @skills run telegram notify "message"')
|
||||
from apps.handlers.notifier import send_telegram_notification
|
||||
from aipass.skills.lib.telegram.apps.handlers.notifier import send_telegram_notification
|
||||
|
||||
message = " ".join(args)
|
||||
success = send_telegram_notification(message)
|
||||
|
||||
@@ -41,14 +41,14 @@ class TestStartAction:
|
||||
assert result["success"] is False
|
||||
assert "bot_id" in result["error"]
|
||||
|
||||
@patch("apps.handlers.bot_operations.start_bot", return_value=0)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_operations.start_bot", return_value=0)
|
||||
def test_start_routes_to_start_bot(self, mock_start):
|
||||
result = run("start", ["base"], {})
|
||||
mock_start.assert_called_once_with("base")
|
||||
assert result["success"] is True
|
||||
assert "base" in result["output"]
|
||||
|
||||
@patch("apps.handlers.bot_operations.start_bot", return_value=None)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_operations.start_bot", return_value=None)
|
||||
def test_start_config_fail_returns_error(self, mock_start):
|
||||
result = run("start", ["missing"], {})
|
||||
assert result["success"] is False
|
||||
@@ -61,43 +61,57 @@ class TestStopAction:
|
||||
assert result["success"] is False
|
||||
assert "bot_id" in result["error"]
|
||||
|
||||
@patch("apps.handlers.bot_operations.stop_bot", return_value=(True, "Stopped telegram-bot@base"))
|
||||
@patch(
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.stop_bot",
|
||||
return_value=(True, "Stopped telegram-bot@base"),
|
||||
)
|
||||
def test_stop_success(self, mock_stop):
|
||||
result = run("stop", ["base"], {})
|
||||
mock_stop.assert_called_once_with("base")
|
||||
assert result["success"] is True
|
||||
|
||||
@patch("apps.handlers.bot_operations.stop_bot", return_value=(False, "Service not found"))
|
||||
@patch(
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.stop_bot", return_value=(False, "Service not found")
|
||||
)
|
||||
def test_stop_failure(self, mock_stop):
|
||||
result = run("stop", ["base"], {})
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
class TestStatusAction:
|
||||
@patch("apps.handlers.bot_operations.get_status", return_value=[])
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_operations.get_status", return_value=[])
|
||||
def test_status_no_bots(self, mock_status):
|
||||
result = run("status", [], {})
|
||||
assert result["success"] is True
|
||||
assert "No bots registered" in result["output"]
|
||||
|
||||
@patch("apps.handlers.bot_operations.get_status", return_value=[])
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_operations.get_status", return_value=[])
|
||||
def test_status_specific_bot_not_found(self, mock_status):
|
||||
result = run("status", ["missing"], {})
|
||||
assert result["success"] is True
|
||||
assert "missing" in result["output"]
|
||||
|
||||
@patch("apps.handlers.bot_operations.format_bot_details", return_value=["Bot ID: base", "Status: running"])
|
||||
@patch("apps.handlers.bot_operations.get_status", return_value=[{"bot_id": "base", "status": "running"}])
|
||||
@patch(
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.format_bot_details",
|
||||
return_value=["Bot ID: base", "Status: running"],
|
||||
)
|
||||
@patch(
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.get_status",
|
||||
return_value=[{"bot_id": "base", "status": "running"}],
|
||||
)
|
||||
def test_status_specific_bot_found(self, mock_status, mock_format):
|
||||
result = run("status", ["base"], {})
|
||||
assert result["success"] is True
|
||||
assert "Bot ID: base" in result["output"]
|
||||
|
||||
@patch(
|
||||
"apps.handlers.bot_operations.format_bot_table",
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.format_bot_table",
|
||||
return_value=["Bot ID Branch Status", "base - running"],
|
||||
)
|
||||
@patch("apps.handlers.bot_operations.get_status", return_value=[{"bot_id": "base"}, {"bot_id": "dev"}])
|
||||
@patch(
|
||||
"aipass.skills.lib.telegram.apps.handlers.bot_operations.get_status",
|
||||
return_value=[{"bot_id": "base"}, {"bot_id": "dev"}],
|
||||
)
|
||||
def test_status_all_bots(self, mock_status, mock_table):
|
||||
result = run("status", [], {})
|
||||
assert result["success"] is True
|
||||
@@ -114,19 +128,19 @@ class TestCreateAction:
|
||||
result = run("create", ["mybot"], {})
|
||||
assert result["success"] is False
|
||||
|
||||
@patch("apps.handlers.bot_factory.create_bot", return_value={"bot_id": "mybot"})
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_factory.create_bot", return_value={"bot_id": "mybot"})
|
||||
def test_create_success(self, mock_create):
|
||||
result = run("create", ["mybot", "123:ABC"], {})
|
||||
mock_create.assert_called_once_with(bot_id="mybot", bot_token="123:ABC", branch_name=None, work_dir=None)
|
||||
assert result["success"] is True
|
||||
assert "mybot" in result["output"]
|
||||
|
||||
@patch("apps.handlers.bot_factory.create_bot", return_value=None)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_factory.create_bot", return_value=None)
|
||||
def test_create_failure(self, mock_create):
|
||||
result = run("create", ["mybot", "123:ABC"], {})
|
||||
assert result["success"] is False
|
||||
|
||||
@patch("apps.handlers.bot_factory.create_bot", return_value={"bot_id": "mybot"})
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_factory.create_bot", return_value={"bot_id": "mybot"})
|
||||
def test_create_with_branch_flag(self, mock_create):
|
||||
result = run("create", ["mybot", "123:ABC", "--branch", "dev"], {})
|
||||
mock_create.assert_called_once_with(bot_id="mybot", bot_token="123:ABC", branch_name="dev", work_dir=None)
|
||||
@@ -139,13 +153,13 @@ class TestDeleteAction:
|
||||
assert result["success"] is False
|
||||
assert "bot_id" in result["error"]
|
||||
|
||||
@patch("apps.handlers.bot_factory.delete_bot", return_value=True)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_factory.delete_bot", return_value=True)
|
||||
def test_delete_success(self, mock_delete):
|
||||
result = run("delete", ["base"], {})
|
||||
mock_delete.assert_called_once_with("base")
|
||||
assert result["success"] is True
|
||||
|
||||
@patch("apps.handlers.bot_factory.delete_bot", return_value=False)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_factory.delete_bot", return_value=False)
|
||||
def test_delete_failure(self, mock_delete):
|
||||
result = run("delete", ["base"], {})
|
||||
assert result["success"] is False
|
||||
@@ -157,20 +171,20 @@ class TestNotifyAction:
|
||||
assert result["success"] is False
|
||||
assert "message" in result["error"]
|
||||
|
||||
@patch("apps.handlers.notifier.send_telegram_notification", return_value=True)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.notifier.send_telegram_notification", return_value=True)
|
||||
def test_notify_success(self, mock_notify):
|
||||
result = run("notify", ["hello", "world"], {})
|
||||
mock_notify.assert_called_once_with("hello world")
|
||||
assert result["success"] is True
|
||||
|
||||
@patch("apps.handlers.notifier.send_telegram_notification", return_value=False)
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.notifier.send_telegram_notification", return_value=False)
|
||||
def test_notify_failure(self, mock_notify):
|
||||
result = run("notify", ["fail"], {})
|
||||
assert result["success"] is False
|
||||
|
||||
|
||||
class TestExceptionHandling:
|
||||
@patch("apps.handlers.bot_operations.get_status", side_effect=RuntimeError("boom"))
|
||||
@patch("aipass.skills.lib.telegram.apps.handlers.bot_operations.get_status", side_effect=RuntimeError("boom"))
|
||||
def test_exception_caught_and_returned(self, mock_status):
|
||||
result = run("status", [], {})
|
||||
assert result["success"] is False
|
||||
|
||||
Reference in New Issue
Block a user