From 63021f84c598b3caade81e6de093f304ebc5f323 Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Fri, 24 Jul 2026 09:50:11 +0100 Subject: [PATCH] patchset: add command to test patches on an existing checkout Add a patchset subcommand that submits patches to the pipeline /api/patchset endpoint, applying them on top of a checkout node created with the checkout command. Patches can be local files uploaded inline or URLs from an allowed domain such as patchwork.kernel.org, with the same job/platform filters and watch options as checkout. Expose the same functionality as KernelCIClient.trigger_patchset and as a trigger_patchset MCP tool, and document the new command. This is linked to kernelci-pipeline#1563 Signed-off-by: Ben Copeland --- docs/_index.md | 6 + docs/patchset.md | 57 ++++++ hugo.toml | 5 + kcidev/api.py | 38 ++++ kcidev/libs/maestro_common.py | 69 ++++++- kcidev/main.py | 2 + kcidev/mcp/tools_maestro.py | 23 +++ kcidev/subcommands/patchset.py | 181 ++++++++++++++++++ kcidev/subcommands/watch.py | 3 +- tests/test_api.py | 54 ++++++ tests/test_kcidev.py | 19 ++ tests/test_libs_and_subcommands_detail.py | 2 +- tests/test_maestro_common.py | 120 ++++++++++++ tests/test_mcp_server.py | 5 + tests/test_patchset.py | 222 ++++++++++++++++++++++ tests/test_watch.py | 60 ++++++ 16 files changed, 858 insertions(+), 8 deletions(-) create mode 100644 docs/patchset.md create mode 100644 kcidev/subcommands/patchset.py create mode 100644 tests/test_patchset.py create mode 100644 tests/test_watch.py diff --git a/docs/_index.md b/docs/_index.md index fa3ea9e6..e3e2379e 100644 --- a/docs/_index.md +++ b/docs/_index.md @@ -107,6 +107,12 @@ Trigger ad-hoc test of specific tree/branch/commit. - [checkout](checkout) +#### patchset + +Test patches on top of an existing checkout. + +- [patchset](patchset) + #### testretry Trigger a test retry for a given Maestro node id. diff --git a/docs/patchset.md b/docs/patchset.md new file mode 100644 index 00000000..2ddd928d --- /dev/null +++ b/docs/patchset.md @@ -0,0 +1,57 @@ ++++ +title = 'patchset' +date = 2026-07-24T00:00:00+01:00 +description = 'Command for testing patches on top of an existing checkout.' ++++ + +This command tests one or more patches on top of an existing checkout on the +KernelCI Pipeline instance. +It is useful to verify a patch or a patch series before sending it to a +mailing list, or to check whether a fix repairs a failing test. + +The patches are applied on top of a checkout node created with the +[checkout](../checkout) command, in the order given. +Patches can be local files, uploaded inline, or URLs from an allowed domain +such as patchwork.kernel.org. + +First trigger a checkout of the tree/branch/commit the patches are based on: +```sh +kci-dev checkout --giturl https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git --branch master --tipoftree +``` + +The command prints a `checkout_nodeid`, which identifies the checkout to +apply the patches on. + +Then test local patch files, for example the output of `git format-patch`: +```sh +kci-dev patchset --nodeid --patch 0001-fix.patch --patch 0002-fix.patch --job-filter "kbuild-gcc-12-x86" +``` + +Or test a patch series from patchwork: +```sh +kci-dev patchset --nodeid --patchurl https://patchwork.kernel.org/series/123456/mbox/ --job-filter "kbuild-gcc-12-x86" +``` + +Where: +- `nodeid` is the id of the checkout node to apply the patches on. +- `patch` is a local patch file to upload. Can be used multiple times. +- `patchurl` is a patch URL from an allowed domain. Can be used multiple + times. `--patch` and `--patchurl` cannot be combined in one request. +- `job-filter` is the job filter to use for the test (optional parameter) +- `platform-filter` is the platform filter to use for the test (optional + parameter) + +Patch files must be text-only unified diffs. The pipeline rejects binary +patches, and at most 32 patches of up to 10 MiB each can be uploaded per +request. + +### --watch, -w + +As with the checkout command, you can use the `--watch` option to watch the +progress of the test on the patched tree, and `--test` to wait for a +particular test result and set the exit code accordingly. +See the [checkout](../checkout) documentation for details. + +```sh +kci-dev patchset --nodeid --patch fix.patch --job-filter baseline-nfs-arm64-qualcomm --watch --test baseline.login +``` diff --git a/hugo.toml b/hugo.toml index a838439c..b957d9bf 100644 --- a/hugo.toml +++ b/hugo.toml @@ -46,6 +46,11 @@ sidebar_cache_limit = 1000 pageRef = "config_file/" weight = 20 + [[menu.main]] + name = "patchset" + pageRef = "patchset/" + weight = 20 + [[menu.main]] name = "testretry" pageRef = "testretry/" diff --git a/kcidev/api.py b/kcidev/api.py index adf29850..5e38303a 100644 --- a/kcidev/api.py +++ b/kcidev/api.py @@ -53,6 +53,7 @@ maestro_get_nodes, send_checkout_full, send_jobretry, + send_patchset, ) from kcidev.main import get_cli @@ -568,3 +569,40 @@ def trigger_checkout( if result is None: raise KciDevError(f"Maestro checkout failed for {giturl} at {commit}") return result + + def trigger_patchset( + self, + nodeid, + patches=None, + patchurls=None, + job_filter=None, + platform_filter=None, + pipeline_url=None, + token=None, + ): + """Test patches on top of an existing checkout node. + + Patches are passed inline as strings via patches, or as URLs from + an allowed domain via patchurls. Exactly one of the two must be + provided. + """ + if bool(patches) == bool(patchurls): + raise KciDevError("Exactly one of patches or patchurls must be provided") + url = pipeline_url or self._instance_setting( + "pipeline", human_readable_key="pipeline URL" + ) + token = token or self._instance_setting("token") + result = _as_library_error( + "Maestro patchset failed", + send_patchset, + url, + token, + nodeid, + patches=patches, + patchurls=patchurls, + job_filter=job_filter, + platform_filter=platform_filter, + ) + if result is None: + raise KciDevError(f"Maestro patchset failed for node {nodeid}") + return result diff --git a/kcidev/libs/maestro_common.py b/kcidev/libs/maestro_common.py index 4479362a..3359f564 100644 --- a/kcidev/libs/maestro_common.py +++ b/kcidev/libs/maestro_common.py @@ -126,7 +126,7 @@ def maestro_get_nodes(url, limit, offset, filter, paginate): return nodes_data -def maestro_check_node(node): +def maestro_check_node(node, root_node="checkout"): """ Node can be defined RUNNING/DONE/FAIL based on the state Simplify, as our current state model suboptimal @@ -139,7 +139,7 @@ def maestro_check_node(node): f"Checking node status - name: {name}, state: {state}, result: {result}" ) - if name == "checkout": + if name == root_node: if state == "running": status = "RUNNING" elif state == "available" or state == "closing": @@ -225,10 +225,10 @@ def maestro_node_result(node): kci_msg(f" - node_id:{node['id']} ({node['updated']})") -def maestro_watch_jobs(baseurl, token, treeid, job_filter, test): - # we need to add to job_filter "checkout" node +def maestro_watch_jobs(baseurl, token, treeid, job_filter, test, root_node="checkout"): + # the tree's root node (checkout or patchset) must complete as well job_filter = list(job_filter) - job_filter.append("checkout") + job_filter.append(root_node) kci_log(f"job_filter: {', '.join(job_filter)}") logging.info(f"Starting job watch for tree {treeid}") logging.debug(f"Watching jobs: {job_filter}, test: {test}") @@ -267,7 +267,7 @@ def maestro_watch_jobs(baseurl, token, treeid, job_filter, test): test_result = node["result"] logging.debug(f"Test node {test} result: {test_result}") if node["name"] in job_filter: - status = maestro_check_node(node) + status = maestro_check_node(node, root_node=root_node) if status == "DONE": if job_info[node["name"]]["running"]: kci_msg("") @@ -399,3 +399,60 @@ def send_checkout_full(baseurl, token, **kwargs): result = response.json() logging.info(f"Checkout successful - tree ID: {result.get('treeid', 'unknown')}") return result + + +def send_patchset( + baseurl, + token, + nodeid, + patches=None, + patchurls=None, + job_filter=None, + platform_filter=None, +): + url = baseurl + "api/patchset" + headers = { + "Content-Type": "application/json; charset=utf-8", + "Authorization": f"{token}", + } + data = {"nodeid": nodeid} + if patches: + data["patch"] = patches + if patchurls: + data["patchurl"] = patchurls + if job_filter: + data["jobfilter"] = job_filter + if platform_filter: + data["platformfilter"] = platform_filter + + logging.info( + "Sending patchset request for checkout node %s with %d inline patches " + "and %d patch URLs", + nodeid, + len(patches or []), + len(patchurls or []), + ) + safe_data = {key: value for key, value in data.items() if key != "patch"} + if patches: + safe_data["patch"] = [{"bytes": len(p.encode("utf-8"))} for p in patches] + logging.debug("Patchset data: %s", safe_data) + + jdata = json.dumps(data) + maestro_print_api_call(url, safe_data) + try: + logging.debug(f"POST request to: {url}") + response = kcidev_session.post(url, headers=headers, data=jdata, timeout=60) + logging.debug(f"Patchset response status: {response.status_code}") + except requests.exceptions.RequestException as e: + logging.error(f"Patchset API request failed: {e}") + kci_err(f"API connection error: {e}") + return None + + if response.status_code != 200: + logging.error(f"Patchset failed with status {response.status_code}") + maestro_api_error(response) + return None + + result = response.json() + logging.info(f"Patchset submitted: {result.get('message', 'No message')}") + return result diff --git a/kcidev/main.py b/kcidev/main.py index bf314b8a..c09d98b6 100755 --- a/kcidev/main.py +++ b/kcidev/main.py @@ -14,6 +14,7 @@ config, maestro, mcp, + patchset, results, storage, submit, @@ -73,6 +74,7 @@ def register_commands(command_group=None): command_group.add_command(config.config) command_group.add_command(maestro.maestro) command_group.add_command(mcp.mcp) + command_group.add_command(patchset.patchset) command_group.add_command(testretry.testretry) command_group.add_command(results.results) command_group.add_command(storage.storage) diff --git a/kcidev/mcp/tools_maestro.py b/kcidev/mcp/tools_maestro.py index e3d70d4f..132586bf 100644 --- a/kcidev/mcp/tools_maestro.py +++ b/kcidev/mcp/tools_maestro.py @@ -81,3 +81,26 @@ def trigger_checkout( return client.trigger_checkout( giturl, branch, commit, job_filter, platform_filter ) + + @server.tool(annotations=action) + @tool_errors + def trigger_patchset( + nodeid: str, + patches: list[str] | None = None, + patchurls: list[str] | None = None, + job_filter: list[str] | None = None, + platform_filter: list[str] | None = None, + ): + """Test patches on top of an existing KernelCI checkout. + + Applies patches to the checkout node given by nodeid (for + example one started with trigger_checkout) and runs the + pipeline on the patched tree. Pass unified diff contents + inline in patches, or URLs from an allowed domain such as + patchwork.kernel.org in patchurls; exactly one of the two + must be provided. Returns the new patchset + node; poll progress with list_nodes using 'treeid='. + """ + return client.trigger_patchset( + nodeid, patches, patchurls, job_filter, platform_filter + ) diff --git a/kcidev/subcommands/patchset.py b/kcidev/subcommands/patchset.py new file mode 100644 index 00000000..38e8297a --- /dev/null +++ b/kcidev/subcommands/patchset.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +import logging +import os +import sys + +import click + +from kcidev.libs.common import * +from kcidev.libs.maestro_common import * + +MAX_INLINE_PATCHES = 32 +MAX_INLINE_PATCH_SIZE = 10 * 1024 * 1024 + + +def load_patch_files(paths): + """Read local patch files, rejecting what the server would refuse.""" + if len(paths) > MAX_INLINE_PATCHES: + raise click.ClickException( + f"Too many patches: {len(paths)} (maximum {MAX_INLINE_PATCHES})" + ) + patches = [] + for path in paths: + size = os.path.getsize(path) + if size > MAX_INLINE_PATCH_SIZE: + raise click.ClickException( + f"{path}: patch too large ({size} bytes, " + f"maximum {MAX_INLINE_PATCH_SIZE})" + ) + with open(path, "rb") as f: + data = f.read() + if not data: + raise click.ClickException(f"{path}: patch file is empty") + if b"\x00" in data: + raise click.ClickException(f"{path}: binary content not allowed") + try: + patches.append(data.decode("utf-8")) + except UnicodeDecodeError: + raise click.ClickException(f"{path}: not valid UTF-8 text") + return patches + + +@click.command( + help="""Test patches on top of an existing KernelCI checkout. + +This command submits a patchset request to KernelCI's pipeline system, +applying one or more patches on top of a checkout node created with the +checkout command. Patches can be local files, uploaded inline, or URLs +from an allowed domain such as patchwork.kernel.org. Patches are applied +in the order given. + +\b +Examples: + # Trigger a checkout first, then test local patches on top of it + kci-dev checkout --giturl https://git.kernel.org/torvalds/linux.git \\ + --branch master --tipoftree + kci-dev patchset --nodeid \\ + --patch 0001-fix.patch --patch 0002-fix.patch + + # Test a patchwork series URL with job and platform filters + kci-dev patchset --nodeid \\ + --patchurl https://patchwork.kernel.org/series/123/mbox/ \\ + --job-filter baseline --platform-filter qemu-x86 + + # Watch for a specific test result and return appropriate exit code + kci-dev patchset --nodeid --patch fix.patch \\ + --job-filter baseline --watch --test baseline.login +""" +) +@click.option( + "--nodeid", + help="Checkout node ID to apply the patches on (from kci-dev checkout)", + required=True, +) +@click.option( + "--patch", + help="Local patch file to upload. Can be used multiple times", + multiple=True, + type=click.Path(exists=True, dir_okay=False), +) +@click.option( + "--patchurl", + help="Patch URL from an allowed domain. Can be used multiple times", + multiple=True, +) +@click.option( + "--job-filter", + help="Filter tests by job name (e.g., baseline, ltp). Can be used multiple times", + multiple=True, +) +@click.option( + "--platform-filter", + help="Filter tests by platform (e.g., qemu-x86, rpi4). Can be used multiple times", + multiple=True, +) +@click.option( + "--watch", + "-w", + help="Watch and display test results interactively as they complete", + is_flag=True, +) +@click.option( + "--test", + help="Watch for specific test result and set exit code accordingly (requires --watch)", +) +@click.pass_context +def patchset(ctx, nodeid, patch, patchurl, job_filter, platform_filter, watch, test): + logging.info(f"Starting patchset command for node: {nodeid}") + logging.debug(f"Patches: {patch}, patch URLs: {patchurl}") + logging.debug( + f"Filters: job_filter={job_filter}, platform_filter={platform_filter}" + ) + logging.debug(f"Options: watch={watch}, test={test}") + + if patch and patchurl: + raise click.UsageError("--patch and --patchurl are mutually exclusive") + if not patch and not patchurl: + raise click.UsageError("Either --patch or --patchurl is required") + if watch and not job_filter: + kci_err("No job filter defined. Can't watch for a job(s)!") + return + if test and not watch: + kci_err("Test option only works with watch option") + return + + cfg = ctx.obj.get("CFG") + instance = ctx.obj.get("INSTANCE") + url = cfg[instance]["pipeline"] + apiurl = cfg[instance]["api"] + token = cfg[instance]["token"] + + logging.debug(f"Using instance: {instance}") + logging.debug(f"Pipeline URL: {url}") + logging.debug(f"API URL: {apiurl}") + + patches = load_patch_files(patch) if patch else None + + resp = send_patchset( + url, + token, + nodeid, + patches=patches, + patchurls=list(patchurl) if patchurl else None, + job_filter=list(job_filter) if job_filter else None, + platform_filter=list(platform_filter) if platform_filter else None, + ) + if not resp: + kci_err("Failed to submit patchset") + sys.exit(64) + + if "message" in resp: + click.secho(resp["message"], fg="green") + + treeid = None + node = resp.get("node") + if node: + treeid = node.get("treeid") + patchset_nodeid = node.get("id") + + logging.info( + f"Patchset submitted successfully - treeid: {treeid}, " + f"nodeid: {patchset_nodeid}" + ) + + if treeid: + click.secho(f"treeid: {treeid}", fg="green") + if patchset_nodeid: + click.secho(f"patchset_nodeid: {patchset_nodeid}", fg="green") + + if watch: + if not treeid: + logging.error("No treeid in response, cannot watch jobs") + kci_err("No treeid returned. Can't watch for a job(s)!") + return + click.secho(f"Watching for jobs on treeid: {treeid}", fg="green") + if test: + click.secho(f"Watching for test result: {test}", fg="green") + maestro_watch_jobs( + apiurl, token, treeid, job_filter, test, root_node="patchset" + ) diff --git a/kcidev/subcommands/watch.py b/kcidev/subcommands/watch.py index e6137692..bf6875bb 100644 --- a/kcidev/subcommands/watch.py +++ b/kcidev/subcommands/watch.py @@ -76,7 +76,8 @@ def watch(ctx, nodeid, job_filter, test): logging.info(f"Found node {nodeid} with tree ID: {tree_id}") logging.info(f"Starting job watch for tree {tree_id}") - maestro_watch_jobs(apiurl, token, tree_id, job_filter, test) + root_node = "patchset" if node.get("name") == "patchset" else "checkout" + maestro_watch_jobs(apiurl, token, tree_id, job_filter, test, root_node=root_node) if __name__ == "__main__": diff --git a/tests/test_api.py b/tests/test_api.py index 89d7ecb0..df5cbf8d 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -116,3 +116,57 @@ def test_trigger_checkout_requires_token(): KernelCIClient(cfg=cfg, instance="test").trigger_checkout( "https://git.example.org/linux.git", "master", "deadbeef", ["baseline"] ) + + +def test_trigger_patchset_posts_payload(monkeypatch): + response = Mock(status_code=200) + response.json.return_value = {"message": "OK", "node": {"treeid": "t1"}} + post = Mock(return_value=response) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + result = _client().trigger_patchset( + "0" * 24, patches=["patch content"], job_filter=["baseline-arm64"] + ) + assert result == {"message": "OK", "node": {"treeid": "t1"}} + assert post.call_args[0][0] == "https://pipeline.example.org/api/patchset" + body = json.loads(post.call_args.kwargs["data"]) + assert body == { + "nodeid": "0" * 24, + "patch": ["patch content"], + "jobfilter": ["baseline-arm64"], + } + + +def test_trigger_patchset_posts_patch_urls(monkeypatch): + response = Mock(status_code=200) + response.json.return_value = {"message": "OK", "node": {}} + post = Mock(return_value=response) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + _client().trigger_patchset( + "0" * 24, + patchurls=["https://patchwork.kernel.org/series/1/mbox/"], + platform_filter=["qemu-arm64"], + ) + body = json.loads(post.call_args.kwargs["data"]) + assert body["patchurl"] == ["https://patchwork.kernel.org/series/1/mbox/"] + assert body["platformfilter"] == ["qemu-arm64"] + + +def test_trigger_patchset_rejects_both_patch_forms(): + with pytest.raises(KciDevError, match="xactly one"): + _client().trigger_patchset( + "0" * 24, + patches=["patch content"], + patchurls=["https://patchwork.kernel.org/series/1/mbox/"], + ) + + +def test_trigger_patchset_requires_a_patch_form(): + with pytest.raises(KciDevError, match="xactly one"): + _client().trigger_patchset("0" * 24) + + +def test_trigger_patchset_failure_raises(monkeypatch): + post = Mock(side_effect=requests.exceptions.ConnectionError("no route")) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + with pytest.raises(KciDevError, match="patchset failed"): + _client().trigger_patchset("0" * 24, patches=["patch content"]) diff --git a/tests/test_kcidev.py b/tests/test_kcidev.py index 44013d1d..f0bd0043 100644 --- a/tests/test_kcidev.py +++ b/tests/test_kcidev.py @@ -88,6 +88,25 @@ def test_kcidev_testretry_help(kcidev_config): assert result.returncode == 0 +def test_kcidev_patchset_help(kcidev_config): + command = [ + "poetry", + "run", + "kci-dev", + "--settings", + kcidev_config, + "patchset", + "--help", + ] + result = run(command, stdout=PIPE, stderr=PIPE, universal_newlines=True) + print("returncode: " + str(result.returncode)) + print("#### stdout ####") + print(result.stdout) + print("#### stderr ####") + print(result.stderr) + assert result.returncode == 0 + + def test_kcidev_checkout_help(kcidev_config): command = [ "poetry", diff --git a/tests/test_libs_and_subcommands_detail.py b/tests/test_libs_and_subcommands_detail.py index df4d5148..ffa2b14d 100644 --- a/tests/test_libs_and_subcommands_detail.py +++ b/tests/test_libs_and_subcommands_detail.py @@ -217,5 +217,5 @@ def test_watch_forwards_filters_to_watch_jobs(monkeypatch): assert result.exit_code == 0, result.output watch_jobs.assert_called_once_with( - "https://api/", "secret", "tree-1", ("baseline",), "login" + "https://api/", "secret", "tree-1", ("baseline",), "login", root_node="checkout" ) diff --git a/tests/test_maestro_common.py b/tests/test_maestro_common.py index c3d646d5..8ef52a71 100644 --- a/tests/test_maestro_common.py +++ b/tests/test_maestro_common.py @@ -1,3 +1,5 @@ +import json +import logging from unittest.mock import Mock import click @@ -14,3 +16,121 @@ def test_maestro_get_node_missing_raises_clean_error(monkeypatch): ) with pytest.raises(click.ClickException, match="not found"): maestro_common.maestro_get_node("https://api.example.org/", "0" * 24) + + +def _response(status_code=200, json_data=None): + response = Mock(status_code=status_code) + response.json.return_value = json_data + return response + + +def test_send_patchset_posts_inline_patches(monkeypatch): + result_json = {"message": "OK", "node": {"id": "n1", "treeid": "t1"}} + post = Mock(return_value=_response(json_data=result_json)) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + result = maestro_common.send_patchset( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patches=["patch zero content", "patch one content"], + job_filter=["baseline-x86"], + ) + assert result == result_json + args, kwargs = post.call_args + assert args[0] == "https://pipeline.example.org/api/patchset" + assert kwargs["headers"]["Authorization"] == "token123" + payload = json.loads(kwargs["data"]) + assert payload["nodeid"] == "0" * 24 + assert payload["patch"] == ["patch zero content", "patch one content"] + assert payload["jobfilter"] == ["baseline-x86"] + assert "patchurl" not in payload + + +def test_send_patchset_posts_patch_urls(monkeypatch): + post = Mock(return_value=_response(json_data={"message": "OK", "node": {}})) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + maestro_common.send_patchset( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patchurls=["https://patchwork.kernel.org/series/1/mbox/"], + platform_filter=["qemu-x86"], + ) + _, kwargs = post.call_args + payload = json.loads(kwargs["data"]) + assert payload["patchurl"] == ["https://patchwork.kernel.org/series/1/mbox/"] + assert payload["platformfilter"] == ["qemu-x86"] + assert "patch" not in payload + assert "jobfilter" not in payload + + +def test_send_patchset_does_not_log_patch_content(monkeypatch, caplog): + post = Mock(return_value=_response(json_data={"message": "OK", "node": {}})) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + with caplog.at_level(logging.DEBUG): + maestro_common.send_patchset( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patches=["SECRET-PATCH-CONTENT"], + ) + assert "SECRET-PATCH-CONTENT" not in caplog.text + + +def test_send_patchset_error_returns_none(monkeypatch): + post = Mock(return_value=_response(status_code=500, json_data={"message": "boom"})) + monkeypatch.setattr(maestro_common.kcidev_session, "post", post) + result = maestro_common.send_patchset( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patches=["patch content"], + ) + assert result is None + + +def test_maestro_check_node_available_patchset_root_is_done(): + node = {"name": "patchset", "state": "available", "result": None} + assert maestro_common.maestro_check_node(node, root_node="patchset") == "DONE" + + +def test_maestro_check_node_available_checkout_still_done_by_default(): + node = {"name": "checkout", "state": "available", "result": None} + assert maestro_common.maestro_check_node(node) == "DONE" + + +def test_maestro_watch_jobs_completes_with_patchset_root(monkeypatch): + nodes = [ + { + "name": "patchset", + "state": "done", + "result": "pass", + "kind": "checkout", + "id": "p1", + "updated": "now", + }, + { + "name": "job1", + "state": "done", + "result": "pass", + "kind": "job", + "id": "j1", + "updated": "now", + }, + ] + monkeypatch.setattr( + maestro_common, "maestro_retrieve_treeid_nodes", Mock(return_value=nodes) + ) + monkeypatch.setattr( + maestro_common.time, + "sleep", + Mock(side_effect=RuntimeError("watch loop did not finish")), + ) + maestro_common.maestro_watch_jobs( + "https://api.example.org/", + "token123", + "t1", + ["job1"], + None, + root_node="patchset", + ) diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 53ee72a3..a7149b3b 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -48,6 +48,7 @@ def test_dashboard_tools_registered_without_config(): assert "get_node" not in tools assert "retry_job" not in tools assert "trigger_checkout" not in tools + assert "trigger_patchset" not in tools def test_maestro_tools_registered_with_full_config(): @@ -56,6 +57,7 @@ def test_maestro_tools_registered_with_full_config(): assert "list_nodes" in tools assert "retry_job" in tools assert "trigger_checkout" in tools + assert "trigger_patchset" in tools def test_action_tools_not_registered_without_token(): @@ -64,6 +66,7 @@ def test_action_tools_not_registered_without_token(): assert "get_node" in tools assert "retry_job" not in tools assert "trigger_checkout" not in tools + assert "trigger_patchset" not in tools def test_action_tools_not_registered_without_pipeline_token(): @@ -77,6 +80,7 @@ def test_action_tools_not_registered_without_pipeline_token(): assert "get_node" in tools assert "retry_job" not in tools assert "trigger_checkout" not in tools + assert "trigger_patchset" not in tools def test_tool_annotations(): @@ -85,6 +89,7 @@ def test_tool_annotations(): assert tools["get_node"].annotations.readOnlyHint is True assert tools["retry_job"].annotations.readOnlyHint is False assert tools["trigger_checkout"].annotations.readOnlyHint is False + assert tools["trigger_patchset"].annotations.readOnlyHint is False def test_call_tool_success(monkeypatch): diff --git a/tests/test_patchset.py b/tests/test_patchset.py new file mode 100644 index 00000000..2e263f20 --- /dev/null +++ b/tests/test_patchset.py @@ -0,0 +1,222 @@ +from unittest.mock import Mock + +import click +import pytest +from click.testing import CliRunner + +from kcidev.subcommands import patchset as patchset_module +from kcidev.subcommands.patchset import load_patch_files, patchset + + +@pytest.fixture +def patch_file(tmp_path): + path = tmp_path / "0001-test.patch" + path.write_text("diff --git a/Makefile b/Makefile\n+EXTRAVERSION = -test\n") + return str(path) + + +def _cli_obj(): + return { + "CFG": { + "test": { + "pipeline": "https://pipeline.example.org/", + "api": "https://api.example.org/", + "token": "token123", + } + }, + "INSTANCE": "test", + } + + +def test_load_patch_files_returns_contents_in_order(tmp_path): + first = tmp_path / "first.patch" + first.write_text("first content") + second = tmp_path / "second.patch" + second.write_text("second content") + assert load_patch_files([str(first), str(second)]) == [ + "first content", + "second content", + ] + + +def test_load_patch_files_rejects_empty_file(tmp_path): + empty = tmp_path / "empty.patch" + empty.write_text("") + with pytest.raises(click.ClickException, match="empty"): + load_patch_files([str(empty)]) + + +def test_load_patch_files_rejects_binary_file(tmp_path): + binary = tmp_path / "binary.patch" + binary.write_bytes(b"diff\x00data") + with pytest.raises(click.ClickException, match="binary"): + load_patch_files([str(binary)]) + + +def test_load_patch_files_rejects_invalid_utf8(tmp_path): + invalid = tmp_path / "invalid.patch" + invalid.write_bytes(b"diff \x80\x81 data") + with pytest.raises(click.ClickException, match="UTF-8"): + load_patch_files([str(invalid)]) + + +def test_load_patch_files_rejects_oversized_file(tmp_path): + big = tmp_path / "big.patch" + big.write_bytes(b"a" * (10 * 1024 * 1024 + 1)) + with pytest.raises(click.ClickException, match="large"): + load_patch_files([str(big)]) + + +def test_load_patch_files_rejects_too_many_files(tmp_path): + paths = [] + for i in range(33): + path = tmp_path / f"{i}.patch" + path.write_text("content") + paths.append(str(path)) + with pytest.raises(click.ClickException, match="[Tt]oo many"): + load_patch_files(paths) + + +def test_patchset_rejects_patch_and_patchurl_together(patch_file): + runner = CliRunner() + result = runner.invoke( + patchset, + [ + "--nodeid", + "0" * 24, + "--patch", + patch_file, + "--patchurl", + "https://patchwork.kernel.org/series/1/mbox/", + ], + obj=_cli_obj(), + ) + assert result.exit_code != 0 + assert "mutually exclusive" in result.output + + +def test_patchset_requires_patch_or_patchurl(): + runner = CliRunner() + result = runner.invoke(patchset, ["--nodeid", "0" * 24], obj=_cli_obj()) + assert result.exit_code != 0 + assert "--patch or --patchurl" in result.output + + +def test_patchset_submits_inline_patches(monkeypatch, patch_file): + send = Mock(return_value={"message": "OK", "node": {"id": "n1", "treeid": "t1"}}) + monkeypatch.setattr(patchset_module, "send_patchset", send) + runner = CliRunner() + result = runner.invoke( + patchset, + [ + "--nodeid", + "0" * 24, + "--patch", + patch_file, + "--job-filter", + "baseline-x86", + ], + obj=_cli_obj(), + ) + assert result.exit_code == 0 + send.assert_called_once_with( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patches=["diff --git a/Makefile b/Makefile\n+EXTRAVERSION = -test\n"], + patchurls=None, + job_filter=["baseline-x86"], + platform_filter=None, + ) + assert "OK" in result.output + assert "t1" in result.output + assert "n1" in result.output + + +def test_patchset_submits_patch_urls(monkeypatch): + send = Mock(return_value={"message": "OK", "node": {"id": "n1", "treeid": "t1"}}) + monkeypatch.setattr(patchset_module, "send_patchset", send) + runner = CliRunner() + result = runner.invoke( + patchset, + [ + "--nodeid", + "0" * 24, + "--patchurl", + "https://patchwork.kernel.org/series/1/mbox/", + ], + obj=_cli_obj(), + ) + assert result.exit_code == 0 + send.assert_called_once_with( + "https://pipeline.example.org/", + "token123", + "0" * 24, + patches=None, + patchurls=["https://patchwork.kernel.org/series/1/mbox/"], + job_filter=None, + platform_filter=None, + ) + + +def test_patchset_failed_submission_exits_64(monkeypatch, patch_file): + monkeypatch.setattr(patchset_module, "send_patchset", Mock(return_value=None)) + runner = CliRunner() + result = runner.invoke( + patchset, + ["--nodeid", "0" * 24, "--patch", patch_file], + obj=_cli_obj(), + ) + assert result.exit_code == 64 + + +def test_patchset_watch_requires_job_filter(patch_file): + runner = CliRunner() + result = runner.invoke( + patchset, + ["--nodeid", "0" * 24, "--patch", patch_file, "--watch"], + obj=_cli_obj(), + ) + assert "job filter" in result.output + + +def test_patchset_test_requires_watch(patch_file): + runner = CliRunner() + result = runner.invoke( + patchset, + ["--nodeid", "0" * 24, "--patch", patch_file, "--test", "baseline.login"], + obj=_cli_obj(), + ) + assert "watch" in result.output + + +def test_patchset_watch_calls_watch_jobs(monkeypatch, patch_file): + send = Mock(return_value={"message": "OK", "node": {"id": "n1", "treeid": "t1"}}) + watch_jobs = Mock() + monkeypatch.setattr(patchset_module, "send_patchset", send) + monkeypatch.setattr(patchset_module, "maestro_watch_jobs", watch_jobs) + runner = CliRunner() + result = runner.invoke( + patchset, + [ + "--nodeid", + "0" * 24, + "--patch", + patch_file, + "--job-filter", + "baseline-x86", + "--watch", + "--test", + "baseline.login", + ], + obj=_cli_obj(), + ) + assert result.exit_code == 0 + watch_jobs.assert_called_once_with( + "https://api.example.org/", + "token123", + "t1", + ("baseline-x86",), + "baseline.login", + root_node="patchset", + ) diff --git a/tests/test_watch.py b/tests/test_watch.py new file mode 100644 index 00000000..fe07cb60 --- /dev/null +++ b/tests/test_watch.py @@ -0,0 +1,60 @@ +from unittest.mock import Mock + +from click.testing import CliRunner + +from kcidev.subcommands import watch as watch_module +from kcidev.subcommands.watch import watch + + +def _cli_obj(): + return { + "CFG": { + "test": { + "pipeline": "https://pipeline.example.org/", + "api": "https://api.example.org/", + "token": "token123", + } + }, + "INSTANCE": "test", + } + + +def _invoke_watch(monkeypatch, node): + monkeypatch.setattr(watch_module, "maestro_get_node", Mock(return_value=node)) + watch_jobs = Mock() + monkeypatch.setattr(watch_module, "maestro_watch_jobs", watch_jobs) + runner = CliRunner() + result = runner.invoke( + watch, + ["--nodeid", "0" * 24, "--job-filter", "job1"], + obj=_cli_obj(), + ) + return result, watch_jobs + + +def test_watch_uses_patchset_root_for_patchset_node(monkeypatch): + node = {"name": "patchset", "treeid": "t1"} + result, watch_jobs = _invoke_watch(monkeypatch, node) + assert result.exit_code == 0 + watch_jobs.assert_called_once_with( + "https://api.example.org/", + "token123", + "t1", + ("job1",), + None, + root_node="patchset", + ) + + +def test_watch_uses_checkout_root_by_default(monkeypatch): + node = {"name": "checkout", "treeid": "t1"} + result, watch_jobs = _invoke_watch(monkeypatch, node) + assert result.exit_code == 0 + watch_jobs.assert_called_once_with( + "https://api.example.org/", + "token123", + "t1", + ("job1",), + None, + root_node="checkout", + )