From 6ea21654b7d7a28ad62ff1b2d43e053131239e2d Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Thu, 27 Aug 2026 13:33:22 -0300 Subject: [PATCH 1/6] [Feature] Add pics-export CLI command (#1092) Add `th-cli test-run-execution pics-export --id ` to fetch the PICS actually used by a test run execution from the backend's new GET /api/v1/test_run_executions/{id}/pics_export endpoint and save it as a zip archive (one PICS XML file per cluster), matching the existing `log --grouped` download pattern. - openapi.json: add the pics_export path (client-generation source). - th_cli/api_lib_autogen/api/test_run_executions_api.py: generated-style async/sync client methods for the new endpoint. - th_cli/commands/test_run_execution.py: new `pics-export` subcommand. Companion to certification-tool-backend#1092. --- openapi.json | 53 ++++++++ tests/test_test_run_execution_pics_export.py | 113 ++++++++++++++++++ .../api/test_run_executions_api.py | 25 ++++ th_cli/commands/test_run_execution.py | 57 +++++++++ 4 files changed, 248 insertions(+) create mode 100644 tests/test_test_run_execution_pics_export.py diff --git a/openapi.json b/openapi.json index b4f07b3..0e2181f 100644 --- a/openapi.json +++ b/openapi.json @@ -1598,6 +1598,59 @@ } } }, + "/api/v1/test_run_executions/{id}/pics_export": { + "get": { + "tags": [ + "test_run_executions" + ], + "summary": "Pics Export", + "description": "Export the PICS actually used by a test run execution.\n\nReturns the PICS that were in effect when the execution ran (the\nexecution's own execution_pics override when set, otherwise the\nproject's PICS at the time of the request), as a zip archive containing\none PICS XML file per cluster. The XML format matches what is already\naccepted by the PUT /projects/{id}/upload_pics endpoint, so the exported\nfiles can be re-imported as-is.\n\nArgs:\n id (int): ID of the TestRunExecution the PICS export is requested for\n\nRaises:\n HTTPException: If there's no TestRunExecution with the given ID\n\nReturns:\n StreamingResponse: .zip file containing one PICS XML file per cluster", + "operationId": "pics_export_api_v1_test_run_executions__id__pics_export_get", + "parameters": [ + { + "required": true, + "schema": { + "title": "Id", + "type": "integer" + }, + "name": "id", + "in": "path" + } + ], + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/zip": { + "schema": { + "type": "string", + "format": "binary" + } + } + }, + "headers": { + "Content-Disposition": { + "description": "Suggests a filename for the downloaded ZIP file", + "schema": { + "type": "string" + }, + "example": "attachment; filename=\"archive.zip\"" + } + } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } + } + } + } + }, "/api/v1/test_run_executions/file_upload/": { "post": { "tags": [ diff --git a/tests/test_test_run_execution_pics_export.py b/tests/test_test_run_execution_pics_export.py new file mode 100644 index 0000000..6ad3ee2 --- /dev/null +++ b/tests/test_test_run_execution_pics_export.py @@ -0,0 +1,113 @@ +# +# Copyright (c) 2026 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Tests for the `test-run-execution pics-export` command.""" + +import os +from unittest.mock import Mock, patch + +import pytest +from click.testing import CliRunner + +from th_cli.api_lib_autogen import models as api_models +from th_cli.api_lib_autogen.exceptions import UnexpectedResponse +from th_cli.commands.test_run_execution import test_run_execution +from th_cli.exceptions import ConfigurationError + + +@pytest.mark.unit +@pytest.mark.cli +class TestPicsExportCommand: + """Test cases for the `test-run-execution pics-export` command.""" + + def test_pics_export_writes_default_filename(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """When no --output-file is given, the file is named after the execution title.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.return_value = b"zip-bytes" + api.read_test_run_execution_api_v1_test_run_executions__id__get.return_value = api_models.TestRunExecution( + id=1, title="My Execution!", state=api_models.TestStateEnum.passed, project_id=1 + ) + + with cli_runner.isolated_filesystem(): + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1"]) + + assert result.exit_code == 0 + assert os.path.exists("MyExecution-pics.zip") + with open("MyExecution-pics.zip", "rb") as f: + assert f.read() == b"zip-bytes" + + api.pics_export_api_v1_test_run_executions__id__pics_export_get.assert_called_once_with(id=1) + + def test_pics_export_writes_custom_output_file(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """When --output-file is given, the export is written there and read_test_run_execution + is not called to derive a filename.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.return_value = b"zip-bytes" + + with cli_runner.isolated_filesystem(): + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1", "--output-file", "out.zip"]) + + assert result.exit_code == 0 + assert os.path.exists("out.zip") + + api.read_test_run_execution_api_v1_test_run_executions__id__get.assert_not_called() + + def test_pics_export_no_content(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """When the export has no content, a message is printed and no file is written.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.return_value = None + + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1"]) + + assert result.exit_code == 0 + assert "No PICS were used for this test run execution." in result.output + + def test_pics_export_configuration_error(self, cli_runner: CliRunner) -> None: + """A ConfigurationError from get_client is surfaced to the user.""" + with patch( + "th_cli.commands.test_run_execution.get_client", + side_effect=ConfigurationError("Could not connect to server"), + ): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1"]) + + assert result.exit_code == 1 + assert "Error: Could not connect to server" in result.output + + def test_pics_export_api_error(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """An UnexpectedResponse from the API is surfaced to the user.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.side_effect = UnexpectedResponse( + status_code=404, + content=b"Test run execution not found", + ) + + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "999"]) + + error_text = ( + "Error: Failed to fetch test run execution PICS export (Status: 404) - Test run execution not found" + ) + assert result.exit_code == 1 + assert error_text in result.output + + def test_pics_export_requires_id(self, cli_runner: CliRunner) -> None: + """The --id parameter is required.""" + result = cli_runner.invoke(test_run_execution, ["pics-export"]) + + assert result.exit_code != 0 + assert "Missing option" in result.output or "--id" in result.output diff --git a/th_cli/api_lib_autogen/api/test_run_executions_api.py b/th_cli/api_lib_autogen/api/test_run_executions_api.py index 464663a..38ed22b 100644 --- a/th_cli/api_lib_autogen/api/test_run_executions_api.py +++ b/th_cli/api_lib_autogen/api/test_run_executions_api.py @@ -287,6 +287,18 @@ def _build_for_download_grouped_log_api_v1_test_run_executions__id__grouped_log_ type_=bytes, method="GET", url="/api/v1/test_run_executions/{id}/grouped-log", path_params=path_params ) + def _build_for_pics_export_api_v1_test_run_executions__id__pics_export_get( + self, id: int + ) -> Coroutine[Any, Any, bytes]: + """ + Pics Export + """ + path_params = {"id": str(id)} + + return self.api_client.request( + type_=bytes, method="GET", url="/api/v1/test_run_executions/{id}/pics_export", path_params=path_params + ) + def _build_for_upload_file_api_v1_test_run_executions_file_upload__post( self, body: m.BodyUploadFileApiV1TestRunExecutionsFileUploadPost ) -> Coroutine[Any, Any, dict[str, Any]]: @@ -502,6 +514,12 @@ async def download_grouped_log_api_v1_test_run_executions__id__grouped_log_get(s """ return await self._build_for_download_grouped_log_api_v1_test_run_executions__id__grouped_log_get(id=id) + async def pics_export_api_v1_test_run_executions__id__pics_export_get(self, id: int) -> bytes: + """ + Pics Export + """ + return await self._build_for_pics_export_api_v1_test_run_executions__id__pics_export_get(id=id) + async def upload_file_api_v1_test_run_executions_file_upload__post( self, body: m.BodyUploadFileApiV1TestRunExecutionsFileUploadPost ) -> dict[str, Any]: @@ -685,6 +703,13 @@ def download_grouped_log_api_v1_test_run_executions__id__grouped_log_get(self, i coroutine = self._build_for_download_grouped_log_api_v1_test_run_executions__id__grouped_log_get(id=id) return get_event_loop().run_until_complete(coroutine) + def pics_export_api_v1_test_run_executions__id__pics_export_get(self, id: int) -> bytes: + """ + Pics Export + """ + coroutine = self._build_for_pics_export_api_v1_test_run_executions__id__pics_export_get(id=id) + return get_event_loop().run_until_complete(coroutine) + def upload_file_api_v1_test_run_executions_file_upload__post( self, body: m.BodyUploadFileApiV1TestRunExecutionsFileUploadPost ) -> dict[str, Any]: diff --git a/th_cli/commands/test_run_execution.py b/th_cli/commands/test_run_execution.py index e53e398..c58915c 100644 --- a/th_cli/commands/test_run_execution.py +++ b/th_cli/commands/test_run_execution.py @@ -219,6 +219,35 @@ def log(id: int, output_file: str, grouped: bool) -> None: raise # Re-raise CLI Errors as-is +@test_run_execution.command( + name="pics-export", + short_help=colorize_help("Export the PICS used by a test run execution"), + help=colorize_cmd_help("pics-export", "Export the PICS actually used by a specific execution"), +) +@click.option( + "--id", + "-i", + required=True, + type=int, + help=colorize_help("Export PICS for the Test Run Execution with this ID"), +) +@click.option( + "--output-file", + "-o", + required=False, + type=str, + help=colorize_help("Output zip file. Test run execution title will be used by default"), +) +def pics_export(id: int, output_file: str) -> None: + try: + with closing(get_client()) as client: + sync_apis = SyncApis(client) + __fetch_test_run_execution_pics_export(sync_apis, id, output_file) + + except CLIError: + raise # Re-raise CLI Errors as-is + + def __test_run_execution_by_id(sync_apis: SyncApis, id: int, json: bool) -> None: try: test_run_execution_api = sync_apis.test_run_executions_api @@ -384,6 +413,34 @@ def __fetch_grouped_test_run_execution_log(sync_apis: SyncApis, id: int, output_ handle_api_error(e, "fetch grouped test run execution log") +def __fetch_test_run_execution_pics_export(sync_apis: SyncApis, id: int, output_file: str | None) -> None: + try: + test_run_execution_api = sync_apis.test_run_executions_api + pics_export_content = test_run_execution_api.pics_export_api_v1_test_run_executions__id__pics_export_get(id=id) + + if pics_export_content: + if not output_file: + execution_data = test_run_execution_api.read_test_run_execution_api_v1_test_run_executions__id__get( + id=id + ) + if execution_data: + import re + + output_file = re.sub(r"[^\w]", "", execution_data.title) + "-pics.zip" + else: + output_file = f"test_run_execution_{id}_pics.zip" + + with open(output_file, "wb") as outfile: + outfile.write(pics_export_content) + + click.echo(f"PICS used for test run execution {id} exported to '{output_file}'") + else: + click.echo("No PICS were used for this test run execution.") + + except UnexpectedResponse as e: + handle_api_error(e, "fetch test run execution PICS export") + + def __print_table_test_executions(test_execution: list) -> None: __print_table_header() if isinstance(test_execution, list): From 1f39848a75f474f5e86ac806d50ca13164e8ced2 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 31 Aug 2026 10:17:02 -0300 Subject: [PATCH 2/6] Update pics-export for backend's 404-on-no-PICS change (#1092) Backend now returns 404 instead of a zero-entry zip when the execution used no PICS; that error already surfaces correctly via the existing UnexpectedResponse handling. Reword the empty-content fallback message so it's not misread as the "no PICS" case, since that path is now unreachable in normal operation. --- tests/test_test_run_execution_pics_export.py | 24 ++++++++++++++++++-- th_cli/commands/test_run_execution.py | 5 +++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/tests/test_test_run_execution_pics_export.py b/tests/test_test_run_execution_pics_export.py index 6ad3ee2..01c528f 100644 --- a/tests/test_test_run_execution_pics_export.py +++ b/tests/test_test_run_execution_pics_export.py @@ -67,7 +67,9 @@ def test_pics_export_writes_custom_output_file(self, cli_runner: CliRunner, mock api.read_test_run_execution_api_v1_test_run_executions__id__get.assert_not_called() def test_pics_export_no_content(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: - """When the export has no content, a message is printed and no file is written.""" + """If the API ever returns an empty-but-successful response, a message is + printed and no file is written (the backend normally 404s instead, see + test_pics_export_no_pics_used_api_error).""" api = mock_sync_apis.test_run_executions_api api.pics_export_api_v1_test_run_executions__id__pics_export_get.return_value = None @@ -75,7 +77,25 @@ def test_pics_export_no_content(self, cli_runner: CliRunner, mock_sync_apis: Moc result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1"]) assert result.exit_code == 0 - assert "No PICS were used for this test run execution." in result.output + assert "No PICS content was returned for this test run execution." in result.output + + def test_pics_export_no_pics_used_api_error(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """When the execution used no PICS, the backend 404s and the CLI surfaces it.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.side_effect = UnexpectedResponse( + status_code=404, + content=b"No PICS were used by this test run execution", + ) + + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + result = cli_runner.invoke(test_run_execution, ["pics-export", "--id", "1"]) + + error_text = ( + "Error: Failed to fetch test run execution PICS export (Status: 404)" + " - No PICS were used by this test run execution" + ) + assert result.exit_code == 1 + assert error_text in result.output def test_pics_export_configuration_error(self, cli_runner: CliRunner) -> None: """A ConfigurationError from get_client is surfaced to the user.""" diff --git a/th_cli/commands/test_run_execution.py b/th_cli/commands/test_run_execution.py index c58915c..24c05dc 100644 --- a/th_cli/commands/test_run_execution.py +++ b/th_cli/commands/test_run_execution.py @@ -435,7 +435,10 @@ def __fetch_test_run_execution_pics_export(sync_apis: SyncApis, id: int, output_ click.echo(f"PICS used for test run execution {id} exported to '{output_file}'") else: - click.echo("No PICS were used for this test run execution.") + # The backend returns 404 (raised as UnexpectedResponse, handled below) when no + # PICS were used, so this only guards against an unexpected empty-but-successful + # response. + click.echo("No PICS content was returned for this test run execution.") except UnexpectedResponse as e: handle_api_error(e, "fetch test run execution PICS export") From 68b7452c5ab156d879aa107e434ffcc762f7095c Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 31 Aug 2026 10:27:16 -0300 Subject: [PATCH 3/6] Pretty-print FastAPI error detail instead of raw dict repr (#1092) handle_api_error() only decoded bytes content, so JSON error bodies (parsed to a dict by UnexpectedResponse.for_response) fell through to str(dict) in the CLI error message, e.g.: Error: ... (Status: 404) - {'detail': 'No PICS were used ...'} Add _format_api_error_content() to unwrap FastAPI's {"detail": ...} shape - a plain string for normal errors, joined into a readable list for 422 validation errors - so every command using handle_api_error() (including the new pics-export) now prints: Error: ... (Status: 404) - No PICS were used by this test run execution --- tests/test_exceptions.py | 39 ++++++++++++++++---- tests/test_test_run_execution_pics_export.py | 7 +++- th_cli/exceptions.py | 22 ++++++++++- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 90a4941..e93d852 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -20,15 +20,8 @@ import click import pytest -from th_cli.exceptions import ( - APIError, - CLIError, - ConfigurationError, - handle_api_error, - handle_file_error, -) from th_cli.api_lib_autogen.exceptions import UnexpectedResponse - +from th_cli.exceptions import APIError, CLIError, ConfigurationError, handle_api_error, handle_file_error # --------------------------------------------------------------------------- # CLIError @@ -182,6 +175,36 @@ def test_status_code_preserved(self): handle_api_error(e, "op") assert exc_info.value.status_code == 422 + def test_dict_content_with_detail_string_is_unwrapped(self): + """FastAPI's {"detail": "..."} bodies should surface just the message, + not the raw dict repr.""" + e = self._make_unexpected_response(404, {"detail": "No PICS were used by this test run execution"}) + with pytest.raises(APIError) as exc_info: + handle_api_error(e, "op") + assert exc_info.value.content == "No PICS were used by this test run execution" + assert "{" not in exc_info.value.format_message() + + def test_dict_content_with_validation_error_list_is_joined(self): + """FastAPI 422 validation errors have detail as a list of error objects.""" + e = self._make_unexpected_response( + 422, + { + "detail": [ + {"loc": ["query", "id"], "msg": "field required", "type": "value_error.missing"}, + {"loc": ["query", "limit"], "msg": "value is not a valid integer", "type": "type_error.integer"}, + ] + }, + ) + with pytest.raises(APIError) as exc_info: + handle_api_error(e, "op") + assert exc_info.value.content == "field required; value is not a valid integer" + + def test_dict_content_without_detail_key_falls_back_to_dict(self): + e = self._make_unexpected_response(500, {"error": "something else"}) + with pytest.raises(APIError) as exc_info: + handle_api_error(e, "op") + assert exc_info.value.content == {"error": "something else"} + # --------------------------------------------------------------------------- # handle_file_error diff --git a/tests/test_test_run_execution_pics_export.py b/tests/test_test_run_execution_pics_export.py index 01c528f..96dde92 100644 --- a/tests/test_test_run_execution_pics_export.py +++ b/tests/test_test_run_execution_pics_export.py @@ -80,11 +80,13 @@ def test_pics_export_no_content(self, cli_runner: CliRunner, mock_sync_apis: Moc assert "No PICS content was returned for this test run execution." in result.output def test_pics_export_no_pics_used_api_error(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: - """When the execution used no PICS, the backend 404s and the CLI surfaces it.""" + """When the execution used no PICS, the backend 404s with a JSON body + (as FastAPI does) and the CLI surfaces the plain detail message, + not the raw dict repr.""" api = mock_sync_apis.test_run_executions_api api.pics_export_api_v1_test_run_executions__id__pics_export_get.side_effect = UnexpectedResponse( status_code=404, - content=b"No PICS were used by this test run execution", + content={"detail": "No PICS were used by this test run execution"}, ) with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): @@ -96,6 +98,7 @@ def test_pics_export_no_pics_used_api_error(self, cli_runner: CliRunner, mock_sy ) assert result.exit_code == 1 assert error_text in result.output + assert "{" not in result.output def test_pics_export_configuration_error(self, cli_runner: CliRunner) -> None: """A ConfigurationError from get_client is surfaced to the user.""" diff --git a/th_cli/exceptions.py b/th_cli/exceptions.py index b38beda..9a69a48 100644 --- a/th_cli/exceptions.py +++ b/th_cli/exceptions.py @@ -15,6 +15,8 @@ # """Custom exceptions and error handling for the CLI.""" +from typing import Any + import click from th_cli.api_lib_autogen.exceptions import UnexpectedResponse @@ -58,12 +60,30 @@ class ConfigurationError(CLIError): pass +def _format_api_error_content(content: Any) -> Any: + """Turn a decoded API error response body into a human-readable string. + + FastAPI error responses are JSON objects, typically {"detail": "..."} + for a plain error or {"detail": [{"loc": [...], "msg": "...", ...}, ...]} + for request validation errors. Fall back to the raw content unchanged + for anything else (e.g. plain text bodies). + """ + if not isinstance(content, dict): + return content + + detail = content.get("detail", content) + if isinstance(detail, list): + messages = [item.get("msg", str(item)) if isinstance(item, dict) else str(item) for item in detail] + return "; ".join(messages) + return detail + + def handle_api_error(e: UnexpectedResponse, operation: str) -> None: """Convert API errors to CLI errors.""" - # Decode bytes content if necessary content = e.content if isinstance(content, bytes): content = content.decode("utf-8", errors="ignore") + content = _format_api_error_content(content) raise APIError(f"Failed to {operation}", status_code=e.status_code, content=content) From d6197e139dc74e25220a2e4fa85c35df5bcdf134 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 31 Aug 2026 11:39:12 -0300 Subject: [PATCH 4/6] Address review feedback: catch write OSError, document 404 (#1092) - pics-export now catches OSError when writing the output file (unwritable directory, permission denied, etc.) and raises a clean CLIError instead of letting a raw traceback surface after an otherwise successful export request. - openapi.json: add the 404 response for pics_export (Test Run Execution not found, or no PICS were used), mirroring the corresponding backend change so the checked-in spec matches what codegen would produce. --- openapi.json | 17 ++++++++++++++++- tests/test_test_run_execution_pics_export.py | 16 ++++++++++++++++ th_cli/commands/test_run_execution.py | 7 +++++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/openapi.json b/openapi.json index 0e2181f..9f64f02 100644 --- a/openapi.json +++ b/openapi.json @@ -1604,7 +1604,7 @@ "test_run_executions" ], "summary": "Pics Export", - "description": "Export the PICS actually used by a test run execution.\n\nReturns the PICS that were in effect when the execution ran (the\nexecution's own execution_pics override when set, otherwise the\nproject's PICS at the time of the request), as a zip archive containing\none PICS XML file per cluster. The XML format matches what is already\naccepted by the PUT /projects/{id}/upload_pics endpoint, so the exported\nfiles can be re-imported as-is.\n\nArgs:\n id (int): ID of the TestRunExecution the PICS export is requested for\n\nRaises:\n HTTPException: If there's no TestRunExecution with the given ID\n\nReturns:\n StreamingResponse: .zip file containing one PICS XML file per cluster", + "description": "Export the PICS actually used by a test run execution.\n\nReturns the PICS that were in effect when the execution ran (the\nexecution's own execution_pics override when set, otherwise the\nproject's PICS at the time of the request), as a zip archive containing\none PICS XML file per cluster. The XML format matches what is already\naccepted by the PUT /projects/{id}/upload_pics endpoint, so the exported\nfiles can be re-imported as-is.\n\nArgs:\n id (int): ID of the TestRunExecution the PICS export is requested for\n\nRaises:\n HTTPException: If there's no TestRunExecution with the given ID, or\n if no PICS were used by the execution\n\nReturns:\n StreamingResponse: .zip file containing one PICS XML file per cluster", "operationId": "pics_export_api_v1_test_run_executions__id__pics_export_get", "parameters": [ { @@ -1638,6 +1638,21 @@ } } }, + "404": { + "description": "Test Run Execution not found, or no PICS were used by it", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "detail": { + "type": "string" + } + } + } + } + } + }, "422": { "description": "Validation Error", "content": { diff --git a/tests/test_test_run_execution_pics_export.py b/tests/test_test_run_execution_pics_export.py index 96dde92..3d6516f 100644 --- a/tests/test_test_run_execution_pics_export.py +++ b/tests/test_test_run_execution_pics_export.py @@ -134,3 +134,19 @@ def test_pics_export_requires_id(self, cli_runner: CliRunner) -> None: assert result.exit_code != 0 assert "Missing option" in result.output or "--id" in result.output + + def test_pics_export_write_failure_raises_cli_error(self, cli_runner: CliRunner, mock_sync_apis: Mock) -> None: + """A file-write failure (e.g. unwritable directory) after a successful export + request is surfaced as a clean CLIError, not a raw traceback.""" + api = mock_sync_apis.test_run_executions_api + api.pics_export_api_v1_test_run_executions__id__pics_export_get.return_value = b"zip-bytes" + + with patch("th_cli.commands.test_run_execution.SyncApis", return_value=mock_sync_apis): + with patch("builtins.open", side_effect=OSError("Permission denied")): + result = cli_runner.invoke( + test_run_execution, ["pics-export", "--id", "1", "--output-file", "/no/such/dir/out.zip"] + ) + + assert result.exit_code == 1 + assert "Failed to write PICS export file '/no/such/dir/out.zip'" in result.output + assert "Permission denied" in result.output diff --git a/th_cli/commands/test_run_execution.py b/th_cli/commands/test_run_execution.py index 24c05dc..8de203b 100644 --- a/th_cli/commands/test_run_execution.py +++ b/th_cli/commands/test_run_execution.py @@ -430,8 +430,11 @@ def __fetch_test_run_execution_pics_export(sync_apis: SyncApis, id: int, output_ else: output_file = f"test_run_execution_{id}_pics.zip" - with open(output_file, "wb") as outfile: - outfile.write(pics_export_content) + try: + with open(output_file, "wb") as outfile: + outfile.write(pics_export_content) + except OSError as e: + raise CLIError(f"Failed to write PICS export file '{output_file}': {e}") click.echo(f"PICS used for test run execution {id} exported to '{output_file}'") else: From e2acac96b37fa0732e988bc99f7fcac30987bec9 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 31 Aug 2026 16:11:35 -0300 Subject: [PATCH 5/6] Preserve field path (loc) in 422 validation error messages (#1092) handle_api_error()'s validation-error-list formatting joined only each error's msg, dropping which field it came from. Two different missing fields produced an ambiguous "field required; field required" message with no way to tell them apart - a regression versus the old raw-dict repr (ugly, but at least complete). Now renders each entry as ": " (dropping the "body" root marker FastAPI adds), matching the same loc-joining approach _format_422_detail() in project.py uses (added in the companion PR #112, landed on v2.16-cli-develop after this branch point - not available here to consolidate onto, but the approach is now shared conceptually). --- tests/test_exceptions.py | 25 +++++++++++++++++++++++-- th_cli/exceptions.py | 21 +++++++++++++++++---- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index e93d852..e08efff 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -185,7 +185,8 @@ def test_dict_content_with_detail_string_is_unwrapped(self): assert "{" not in exc_info.value.format_message() def test_dict_content_with_validation_error_list_is_joined(self): - """FastAPI 422 validation errors have detail as a list of error objects.""" + """FastAPI 422 validation errors have detail as a list of error objects, + rendered as ": " per entry.""" e = self._make_unexpected_response( 422, { @@ -197,7 +198,27 @@ def test_dict_content_with_validation_error_list_is_joined(self): ) with pytest.raises(APIError) as exc_info: handle_api_error(e, "op") - assert exc_info.value.content == "field required; value is not a valid integer" + assert exc_info.value.content == "query.id: field required; query.limit: value is not a valid integer" + + def test_dict_content_with_validation_error_list_distinguishes_same_message(self): + """Two different fields failing with the SAME message must still be + distinguishable by field path - joining bare msgs alone would collapse + them into an ambiguous, indistinguishable string.""" + e = self._make_unexpected_response( + 422, + { + "detail": [ + {"loc": ["body", "config", "th_config", "timeout"], "msg": "field required"}, + {"loc": ["body", "config", "network", "wifi", "ssid"], "msg": "field required"}, + ] + }, + ) + with pytest.raises(APIError) as exc_info: + handle_api_error(e, "op") + assert ( + exc_info.value.content + == "config.th_config.timeout: field required; config.network.wifi.ssid: field required" + ) def test_dict_content_without_detail_key_falls_back_to_dict(self): e = self._make_unexpected_response(500, {"error": "something else"}) diff --git a/th_cli/exceptions.py b/th_cli/exceptions.py index 9a69a48..3797235 100644 --- a/th_cli/exceptions.py +++ b/th_cli/exceptions.py @@ -65,16 +65,29 @@ def _format_api_error_content(content: Any) -> Any: FastAPI error responses are JSON objects, typically {"detail": "..."} for a plain error or {"detail": [{"loc": [...], "msg": "...", ...}, ...]} - for request validation errors. Fall back to the raw content unchanged - for anything else (e.g. plain text bodies). + for request validation errors. Each list entry is rendered as + ": " (e.g. "config.th_config.timeout: value is not + a valid integer") so which field failed isn't lost when there are + multiple errors. Falls back to the raw content unchanged for anything + else (e.g. plain text bodies). """ if not isinstance(content, dict): return content detail = content.get("detail", content) if isinstance(detail, list): - messages = [item.get("msg", str(item)) if isinstance(item, dict) else str(item) for item in detail] - return "; ".join(messages) + lines = [] + for error in detail: + if not isinstance(error, dict): + lines.append(str(error)) + continue + # "body" is FastAPI's marker for the request body root; drop it + # so paths read as e.g. "config.th_config.timeout" rather than + # "body.config.th_config.timeout". + loc = ".".join(str(part) for part in error.get("loc", []) if part != "body") + msg = error.get("msg", "") + lines.append(f"{loc}: {msg}" if loc else msg) + return "; ".join(lines) if lines else str(detail) return detail From 73e246b6bdde08ee1d28d507fe591fce0ad6d8b8 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Tue, 1 Sep 2026 11:51:38 +0000 Subject: [PATCH 6/6] Fixes after generate_client script execution --- th_cli/api_lib_autogen/api_client.py | 12 ++++++++---- th_cli/api_lib_autogen/models.py | 16 +++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/th_cli/api_lib_autogen/api_client.py b/th_cli/api_lib_autogen/api_client.py index 4d7d04c..61674d4 100644 --- a/th_cli/api_lib_autogen/api_client.py +++ b/th_cli/api_lib_autogen/api_client.py @@ -80,12 +80,14 @@ def close(self) -> None: @overload async def request( self, *, type_: Type[T], method: str, url: str, path_params: dict[str, Any] | None = None, **kwargs: Any - ) -> T: ... + ) -> T: + ... @overload async def request( self, *, type_: None, method: str, url: str, path_params: dict[str, Any] | None = None, **kwargs: Any - ) -> None: ... + ) -> None: + ... async def request( self, *, type_: Any, method: str, url: str, path_params: dict[str, Any] | None = None, **kwargs: Any @@ -97,10 +99,12 @@ async def request( return await self.send(request, type_) @overload - def request_sync(self, *, type_: Type[T], **kwargs: Any) -> T: ... + def request_sync(self, *, type_: Type[T], **kwargs: Any) -> T: + ... @overload - def request_sync(self, *, type_: None, **kwargs: Any) -> None: ... + def request_sync(self, *, type_: None, **kwargs: Any) -> None: + ... def request_sync(self, *, type_: Any, **kwargs: Any) -> Any: """ diff --git a/th_cli/api_lib_autogen/models.py b/th_cli/api_lib_autogen/models.py index 31a9e68..774f704 100644 --- a/th_cli/api_lib_autogen/models.py +++ b/th_cli/api_lib_autogen/models.py @@ -1,6 +1,6 @@ # generated by datamodel-codegen: -# filename: tmp1nhhp82y.json -# timestamp: 2026-07-07T19:42:35+00:00 +# filename: tmpktlfxzkc.json +# timestamp: 2026-09-01T11:49:15+00:00 from __future__ import annotations @@ -79,6 +79,11 @@ class PICSItem(BaseModel): enabled: Annotated[bool, Field(title="Enabled")] +class THConfig(BaseModel): + prompt_timeout_seconds: Annotated[int | None, Field(title="Prompt Timeout Seconds")] = 60 + enable_realtime_python_test_logs: Annotated[bool | None, Field(title="Enable Realtime Python Test Logs")] = None + + class PublicId(RootModel[str]): root: Annotated[str, Field(title="Public Id")] @@ -124,6 +129,7 @@ class TestCaseMetadataBase(BaseModel): class TestEnvironmentConfig(BaseModel): test_parameters: Annotated[dict[str, Any] | None, Field(title="Test Parameters")] = None + th_config: THConfig | None = None class TestHarnessBackendVersion(BaseModel): @@ -438,9 +444,9 @@ class TestRunExecutionToExport(BaseModel): started_at: Annotated[datetime | None, Field(title="Started At")] = None completed_at: Annotated[datetime | None, Field(title="Completed At")] = None archived_at: Annotated[datetime | None, Field(title="Archived At")] = None - test_suite_executions: Annotated[list[TestSuiteExecutionToExport] | None, Field(title="Test Suite Executions")] = ( - None - ) + test_suite_executions: Annotated[ + list[TestSuiteExecutionToExport] | None, Field(title="Test Suite Executions") + ] = None created_at: Annotated[datetime, Field(title="Created At")] log: Annotated[list[TestRunLogEntry], Field(title="Log")] operator: OperatorToExport | None = None