Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from hackbot_runtime import HackbotContext, run_async
from hackbot_runtime.actions.testrail import record_test_plan
from pydantic_settings import BaseSettings, SettingsConfigDict

from .agent import TestPlanGeneratorResult, run_test_plan_generator
Expand All @@ -21,7 +22,7 @@ async def main(ctx: HackbotContext) -> TestPlanGeneratorResult:

firefox_path = str(install_firefox_nightly())

return await run_test_plan_generator(
result = await run_test_plan_generator(
feature_name=inputs.feature_name,
feature_description=inputs.feature_description,
test_scope=inputs.test_scope,
Expand All @@ -32,6 +33,10 @@ async def main(ctx: HackbotContext) -> TestPlanGeneratorResult:
log=ctx.log_path,
verbose=True,
)
if result.result is None:
raise RuntimeError("Cannot submit an empty test plan to TestRail")
record_test_plan(ctx.actions, result.result.model_dump(mode="json"))
return result


if __name__ == "__main__":
Expand Down
10 changes: 8 additions & 2 deletions libs/hackbot-runtime/hackbot_runtime/actions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@
claude-sdk adapter is ``hackbot_runtime.actions.claude_sdk.actions_server_for``.
"""

from hackbot_runtime.actions import bugzilla, phabricator
from hackbot_runtime.actions import bugzilla, phabricator, testrail
from hackbot_runtime.actions.recorder import ActionsRecorder

ACTIONS_SERVER_NAME = "actions"

__all__ = ["ACTIONS_SERVER_NAME", "ActionsRecorder", "bugzilla", "phabricator"]
__all__ = [
"ACTIONS_SERVER_NAME",
"ActionsRecorder",
"bugzilla",
"phabricator",
"testrail",
]
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
AddCommentHandler as PhabricatorAddCommentHandler,
)
from hackbot_runtime.actions.handlers.phabricator_handler import SubmitPatchHandler
from hackbot_runtime.actions.handlers.testrail_handler import SubmitTestCasesHandler

# Maps a recorded action's dotted `type` to the handler that applies it.
# Adding a new action type later is a one-line addition here — the dispatch
Expand All @@ -20,6 +21,7 @@
"bugzilla.create_bug": CreateBugHandler(),
"phabricator.submit_patch": SubmitPatchHandler(),
"phabricator.add_comment": PhabricatorAddCommentHandler(),
"testrail.submit_test_cases": SubmitTestCasesHandler(),
}


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
"""Apply-side TestRail action for generated test cases."""

from __future__ import annotations

import logging
import os
from typing import Any

import requests

from hackbot_runtime.actions.handlers.base import ActionResult, ApplyContext

log = logging.getLogger(__name__)

_DEFAULT_TESTRAIL_URL = "https://mozilla.testrail.io"
_CASE_TYPE_NAME = "Functional"
_CASE_TEMPLATE_NAME = "Test Case (Steps)"
_CASE_LABEL = "AI Generated"
_SECTION_NAME = "Test Cases"
_TIMEOUT_SECONDS = 30


def _required_env(name: str) -> str:
value = os.environ.get(name, "").strip()
if not value:
raise RuntimeError(f"{name} is not configured")
return value


def _base_url() -> str:
return os.environ.get("TESTRAIL_URL", _DEFAULT_TESTRAIL_URL).rstrip("/")


def _required_int_env(name: str) -> int:
raw_value = _required_env(name)
try:
return int(raw_value)
except ValueError as exc:
raise RuntimeError(f"{name} must be an integer") from exc


def _project_id() -> int:
return _required_int_env("TESTRAIL_PROJECT_ID")


def _api_url(endpoint: str) -> str:
return f"{_base_url()}/index.php?/api/v2/{endpoint.lstrip('/')}"


def _api_request(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us move the part to communicate with TestRail api into its own lib (i.e., libs/testrail-client) like we do for Phabricator (libs/phabricator-client). We could follow that as a reference design.

We keep here the business logic only.

method: str, endpoint: str, data: dict[str, Any] | None = None
) -> dict[str, Any] | list[Any]:
response = requests.request(
method,
_api_url(endpoint),
auth=(
_required_env("TESTRAIL_USERNAME"),
_required_env("TESTRAIL_API_KEY"),
),
json=data,
headers={"Content-Type": "application/json"},
timeout=_TIMEOUT_SECONDS,
)
response.raise_for_status()
if not response.content:
return {}
result = response.json()
if not isinstance(result, (dict, list)):
raise RuntimeError("TestRail returned an unexpected response")
return result


def _require_id(response: object, object_name: str) -> int:
if not isinstance(response, dict):
raise RuntimeError(f"TestRail did not return a {object_name} object")
value = response.get("id")
try:
return int(value)
except (TypeError, ValueError) as exc:
raise RuntimeError(
f"TestRail did not return an id for the created {object_name}"
) from exc


def _resolve_case_type_id() -> int:
response = _api_request("GET", "get_case_types")
case_types = (
response.get("case_types", []) if isinstance(response, dict) else response
)
for case_type in case_types:
if (
isinstance(case_type, dict)
and str(case_type.get("name", "")).strip().casefold()
== _CASE_TYPE_NAME.casefold()
):
return _require_id(case_type, "case type")
raise RuntimeError(f'TestRail has no case type named "{_CASE_TYPE_NAME}"')


def _resolve_template_id(project_id: int) -> int:
response = _api_request("GET", f"get_templates/{project_id}")
templates = (
response.get("templates", []) if isinstance(response, dict) else response
)
for template in templates:
if (
isinstance(template, dict)
and str(template.get("name", "")).strip().casefold()
== _CASE_TEMPLATE_NAME.casefold()
):
return _require_id(template, "template")
raise RuntimeError(f'TestRail has no template named "{_CASE_TEMPLATE_NAME}"')


def _case_payload(
test_case: dict[str, Any], case_type_id: int, template_id: int
) -> dict[str, Any]:
steps = [str(step) for step in test_case.get("steps", [])]
payload: dict[str, Any] = {
"title": test_case["title"],
"type_id": case_type_id,
"template_id": template_id,
"labels": [_CASE_LABEL],
"custom_steps_separated": [{"content": step, "expected": ""} for step in steps],
}
if test_case.get("preconditions"):
payload["custom_preconds"] = test_case["preconditions"]
return payload


def _suite_url(suite_id: int) -> str:
return f"{_base_url()}/index.php?/suites/view/{suite_id}"


class SubmitTestCasesHandler:
async def apply(self, params: dict[str, Any], ctx: ApplyContext) -> ActionResult:
feature = str(params.get("feature") or "").strip()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a test case handler, but it's currently handling test plans too. :p

I still think it'd be better to allow adding cases one by one. We could have a separate method to create a suite, and another to create a section; each taking a ref. Test cases would then reference that ref, making it available when the cases get applied, so we can retrieve their IDs once they're created and use them to create the cases.

test_cases = params.get("generated_test_cases") or []

if not feature:
return ActionResult.failed("TestRail submission requires a feature name")
if not test_cases:
return ActionResult.failed("TestRail submission requires test cases")

try:
project_id = _project_id()
case_type_id = _resolve_case_type_id()
template_id = _resolve_template_id(project_id)
suite_name = f"[Hackbot] - {feature}"
suite_id = _require_id(
_api_request(
"POST",
f"add_suite/{project_id}",
{"name": suite_name},
),
"suite",
)
section_id = _require_id(
_api_request(
"POST",
f"add_section/{project_id}",
{"suite_id": suite_id, "name": _SECTION_NAME},
),
"section",
)

created_case_ids: dict[int, int] = {}
for test_case in test_cases:
generated_id = int(test_case["id"])
case_id = _require_id(
_api_request(
"POST",
f"add_case/{section_id}",
_case_payload(test_case, case_type_id, template_id),
),
"case",
)
created_case_ids[generated_id] = case_id

except Exception as exc:
log.exception(
"Failed to submit test plan to TestRail for run %s", ctx.run_id
)
return ActionResult.failed(str(exc))

return ActionResult.ok(
{
"suite_id": suite_id,
"suite_url": _suite_url(suite_id),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using url instead of suite_url would avoid know issues, see #6330

Suggested change
"suite_url": _suite_url(suite_id),
"url": _suite_url(suite_id),

"section_id": section_id,
"case_ids": list(created_case_ids.values()),
}
)
28 changes: 28 additions & 0 deletions libs/hackbot-runtime/hackbot_runtime/actions/testrail.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
"""TestRail-domain recordable actions.

The test plan generator records this action deterministically after its
structured result has been validated. The external TestRail mutation still
happens only in the apply side handler.
"""

from __future__ import annotations

from typing import Any

from hackbot_runtime.actions.recorder import ActionsRecorder

ACTION_TYPE = "testrail.submit_test_cases"


def record_test_plan(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this working? I expect not.

Could you please structure it like libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py?

recorder: ActionsRecorder,
test_plan: dict[str, Any],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use pydantic to annotate the inputs like in actions/phabricator.py. That will be provided to the agent to know ho to use the tool.

*,
reasoning: str = "Upload the generated test cases to TestRail.",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No point in having a default value here. If we have them test case by test case, then having the agent providing reasoning would make sense, but for the test plan itself, it is kind of useless, and we could drop it.

) -> dict:
"""Record validated generated cases for deferred TestRail submission."""
params = {
"feature": test_plan["feature"],
"generated_test_cases": test_plan["generated_test_cases"],
}
return recorder.record(ACTION_TYPE, params, reasoning=reasoning)
22 changes: 22 additions & 0 deletions libs/hackbot-runtime/tests/test_testrail_action.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from hackbot_runtime.actions import ActionsRecorder
from hackbot_runtime.actions.handlers import get_handler
from hackbot_runtime.actions.handlers.testrail_handler import SubmitTestCasesHandler
from hackbot_runtime.actions.testrail import ACTION_TYPE, record_test_plan


def test_record_test_plan_records_deferred_action():
recorder = ActionsRecorder()
plan = {"feature": "Feature", "generated_test_cases": [], "results": []}

action = record_test_plan(recorder, plan)

assert action["type"] == ACTION_TYPE
assert action["params"] == {
"feature": "Feature",
"generated_test_cases": [],
}
assert recorder.actions == [action]


def test_submit_test_cases_handler_is_registered():
assert isinstance(get_handler(ACTION_TYPE), SubmitTestCasesHandler)
Loading