diff --git a/.basedpyright/baseline.bfabric.json b/.basedpyright/baseline.bfabric.json index 8314f0cb8..3620b9af0 100644 --- a/.basedpyright/baseline.bfabric.json +++ b/.basedpyright/baseline.bfabric.json @@ -235,46 +235,6 @@ "lineCount": 1 } }, - { - "code": "reportAny", - "range": { - "startColumn": 8, - "endColumn": 16, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 19, - "endColumn": 31, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 12, - "endColumn": 20, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 8, - "endColumn": 16, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 19, - "endColumn": 33, - "lineCount": 1 - } - }, { "code": "reportUnknownMemberType", "range": { diff --git a/bfabric/docs/changelog.md b/bfabric/docs/changelog.md index 924187497..b44c9df65 100644 --- a/bfabric/docs/changelog.md +++ b/bfabric/docs/changelog.md @@ -11,7 +11,11 @@ Minor breaking changes are still possible in `1.X.Y` but we try to announce them ### Fixed +- An unknown query field (e.g. `status` on an endpoint without it), a SOAP server fault, or a connection lost mid-request now raise `BfabricRequestError` / `BfabricUnavailableError` instead of raw suds exceptions, so `bfabric-cli` reports them in one line instead of a traceback. +- `use_client` reports a `ValueError`, `OSError`, YAML error or HTTP error status from the command or from loading the config (e.g. no config file yet) as one `Error:` line instead of a traceback; the new `report_user_errors` context manager applies the same handling elsewhere. +- An unknown config environment (e.g. a typo in `--config-env`) raises a `ValueError` listing the available environments instead of a `KeyError`. - OAuth requests to an instance recorded with an `http://` base URL are sent over `https://` instead of failing on the server's redirect (loopback hosts excepted). `bfabric.oauth.https_base_url` exposes the upgrade. +- On a CPU without the features polars needs (e.g. AVX2), importing polars through `bfabric` raises polars' `RuntimeWarning` telling you to install `polars[rtcompat]`, instead of the process dying with "Illegal instruction". ### Added diff --git a/bfabric/src/bfabric/__init__.py b/bfabric/src/bfabric/__init__.py index 6016a589a..377c0ed5d 100644 --- a/bfabric/src/bfabric/__init__.py +++ b/bfabric/src/bfabric/__init__.py @@ -1,10 +1,14 @@ import importlib.metadata +import warnings from bfabric.bfabric import Bfabric from bfabric.config.base_url import BaseUrl from bfabric.config.bfabric_auth import BfabricAuth from bfabric.config.bfabric_client_config import BfabricAPIEngineType, BfabricClientConfig +# polars only warns about missing CPU features and then dies with SIGILL; raise its install hint instead +warnings.filterwarnings("error", message="Missing required CPU features", category=RuntimeWarning) + __all__ = [ "BaseUrl", "Bfabric", diff --git a/bfabric/src/bfabric/config/config_data.py b/bfabric/src/bfabric/config/config_data.py index 6ac011237..2a3649323 100644 --- a/bfabric/src/bfabric/config/config_data.py +++ b/bfabric/src/bfabric/config/config_data.py @@ -93,7 +93,7 @@ def _load_environment_config_data(config_path: Path | str, force_config_env: str logger.debug(f"Reading configuration from: {config_file_path} {config_env=}") config_file = ConfigFile.model_validate(yaml.safe_load(config_file_path.read_text())) resolved_env = config_file.get_selected_config_env(explicit_config_env=config_env) - env_config = config_file.environments[resolved_env] + env_config = config_file.get_selected_config(explicit_config_env=resolved_env) return ConfigData( client=env_config.config, auth=env_config.auth, diff --git a/bfabric/src/bfabric/config/config_file.py b/bfabric/src/bfabric/config/config_file.py index 6db196024..b14a51784 100644 --- a/bfabric/src/bfabric/config/config_file.py +++ b/bfabric/src/bfabric/config/config_file.py @@ -136,8 +136,15 @@ def get_selected_config_env(self, explicit_config_env: str | None) -> str: return env def get_selected_config(self, explicit_config_env: str | None = None) -> EnvironmentConfig: - """Return the selected environment; see :meth:`get_selected_config_env`.""" - return self.environments[self.get_selected_config_env(explicit_config_env=explicit_config_env)] + """Return the selected environment; see :meth:`get_selected_config_env`. + + :raises ValueError: if no environment of that name is in the config + """ + name = self.get_selected_config_env(explicit_config_env=explicit_config_env) + if name not in self.environments: + available = ", ".join(sorted(self.environments)) + raise ValueError(f"Environment {name!r} not found in the config; available: {available}") + return self.environments[name] def read_config_file( diff --git a/bfabric/src/bfabric/engine/engine_suds.py b/bfabric/src/bfabric/engine/engine_suds.py index 9d016f984..5adf091f7 100644 --- a/bfabric/src/bfabric/engine/engine_suds.py +++ b/bfabric/src/bfabric/engine/engine_suds.py @@ -1,11 +1,12 @@ from __future__ import annotations import copy +from contextlib import contextmanager from typing import TYPE_CHECKING, Any from urllib.error import URLError import suds.transport -from suds import MethodNotFound +from suds import MethodNotFound, TypeNotFound, WebFault from suds.client import Client from bfabric.engine.response_format_suds import suds_asdict_recursive @@ -15,6 +16,8 @@ from bfabric.results.result_container import ResultContainer if TYPE_CHECKING: + from collections.abc import Iterator + from suds.serviceproxy import ServiceProxy from bfabric.config import BaseUrl, BfabricAuth @@ -58,7 +61,8 @@ def read( "idonly": return_id_only, } service = self._get_suds_service(endpoint) - response = service.read(full_query) + with self._translate_call_errors(endpoint): + response = service.read(full_query) # pyright: ignore[reportAny] return self._convert_results(response=response, endpoint=endpoint) def save( @@ -78,9 +82,11 @@ def save( query = {"login": auth.login, "password": auth.password.get_secret_value(), endpoint: obj} service = self._get_suds_service(endpoint) try: - response = getattr(service, method)(query) + save_method = getattr(service, method) # pyright: ignore[reportAny] except MethodNotFound as e: raise BfabricRequestError(f"SUDS failed to find save method for the {endpoint} endpoint.") from e + with self._translate_call_errors(endpoint): + response = save_method(query) # pyright: ignore[reportAny] return self._convert_results(response=response, endpoint=endpoint) def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> ResultContainer: @@ -93,7 +99,8 @@ def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> Resul return ResponseDelete.from_empty_request() query = {"login": auth.login, "password": auth.password.get_secret_value(), "id": id} service = self._get_suds_service(endpoint) - response = service.delete(query) + with self._translate_call_errors(endpoint): + response = service.delete(query) # pyright: ignore[reportAny] return ResponseDelete.from_suds(suds_response=response, endpoint=endpoint) def _get_suds_service(self, endpoint: str) -> ServiceProxy: @@ -116,6 +123,27 @@ def _get_suds_service(self, endpoint: str) -> ServiceProxy: raise BfabricUnavailableError(self._base_url, error) from error return self._cl[endpoint].service + @contextmanager + def _translate_call_errors(self, endpoint: str) -> Iterator[None]: + """Translate the suds exceptions a service call raises into errors the callers handle. + + None of them is a ``RuntimeError``, so untranslated a typo in a query field (``TypeNotFound``) + escapes the CLI's error handling as a traceback. suds reports an HTTP error status (e.g. a proxy's + 502) as a bare ``Exception((status, reason))``, hence the catch-all. + """ + try: + yield + except TypeNotFound as error: + # suds keeps the name only in its message, "Type not found: ''" + name = str(error).removeprefix("Type not found: ") + raise BfabricRequestError(f"Unknown field {name} for the {endpoint!r} endpoint.") from error + except WebFault as error: + raise BfabricRequestError(f"B-Fabric server fault on the {endpoint!r} endpoint: {error}") from error + except OSError as error: + raise BfabricUnavailableError(self._base_url, error) from error + except Exception as error: + raise BfabricRequestError(f"Request to the {endpoint!r} endpoint failed: {error}") from error + def _convert_results(self, response: Any, endpoint: str) -> ResultContainer: try: n_available_pages = response["numberofpages"] diff --git a/bfabric/src/bfabric/entities/application.py b/bfabric/src/bfabric/entities/application.py index 52853bda6..a8d55d93e 100644 --- a/bfabric/src/bfabric/entities/application.py +++ b/bfabric/src/bfabric/entities/application.py @@ -31,7 +31,7 @@ def technology_folder_name(self) -> PathSafeStr: if not _is_technology_list(technology_raw): raise ValueError("Technology must be a list of strings") technology = sorted(technology_raw)[0] - return TypeAdapter(PathSafeStr).validate_python(technology) + return TypeAdapter(PathSafeStr).validate_python(technology) # pyright: ignore[reportArgumentType] def _is_technology_list(technology: ApiResponseDataType) -> TypeGuard[list[str]]: diff --git a/bfabric/src/bfabric/entities/core/uri.py b/bfabric/src/bfabric/entities/core/uri.py index a35eceabf..a7dadc2de 100644 --- a/bfabric/src/bfabric/entities/core/uri.py +++ b/bfabric/src/bfabric/entities/core/uri.py @@ -68,7 +68,9 @@ def invalid(reason: str) -> ValueError: ValidatedEntityUri = Annotated[str, AfterValidator(_validate_entity_uri)] -entity_uri_adapter: TypeAdapter[str] = TypeAdapter(ValidatedEntityUri) +entity_uri_adapter: TypeAdapter[str] = TypeAdapter( # pyright: ignore[reportUnknownVariableType] + ValidatedEntityUri # pyright: ignore[reportArgumentType] +) class EntityUri(str): diff --git a/bfabric/src/bfabric/utils/cli_integration.py b/bfabric/src/bfabric/utils/cli_integration.py index f18db3baf..69f635e9c 100644 --- a/bfabric/src/bfabric/utils/cli_integration.py +++ b/bfabric/src/bfabric/utils/cli_integration.py @@ -5,9 +5,12 @@ import os import sys import traceback +from contextlib import contextmanager from pathlib import Path from typing import TYPE_CHECKING, Annotated, NoReturn, TypeVar, cast +import httpx +import yaml from loguru import logger from rich.highlighter import RegexHighlighter from rich.theme import Theme @@ -15,7 +18,7 @@ from bfabric.config import DEFAULT_CONFIG_FILE if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Iterator T = TypeVar("T") @@ -69,22 +72,28 @@ def wrapper(*args: object, **kwargs: object) -> T: if "client" in kwargs: client = cast("Bfabric", kwargs.pop("client")) else: - try: + with report_user_errors(): client = Bfabric.connect( config_file_path=config_file or DEFAULT_CONFIG_FILE, config_file_env=config_env or "default", ) - except (ValueError, RuntimeError) as e: - _report_and_exit(e) - try: + with report_user_errors(): return fn(*args, client=client, **kwargs) # type: ignore[arg-type] - except RuntimeError as e: - _report_and_exit(e) wrapper.__signature__ = new_sig # type: ignore[reportAttributeAccessIssue] return wrapper +@contextmanager +def report_user_errors() -> Iterator[None]: + """Report an error a user can cause (bad argument, missing file, unreadable config, entity not found, + HTTP error status) as one stderr line and exit 1.""" + try: + yield + except (RuntimeError, ValueError, OSError, yaml.YAMLError, httpx.HTTPStatusError) as e: + _report_and_exit(e) + + def _report_and_exit(error: Exception) -> NoReturn: """Reports ``error`` as a single stderr line and exits 1, keeping the traceback for DEBUG runs.""" # Not logger.opt(exception=...): the DEBUG sink runs with loguru's default diagnose=True, which diff --git a/bfabric_asgi_auth/pyproject.toml b/bfabric_asgi_auth/pyproject.toml index 0809673ae..c9c21091c 100644 --- a/bfabric_asgi_auth/pyproject.toml +++ b/bfabric_asgi_auth/pyproject.toml @@ -27,7 +27,7 @@ demo = [ ] test = [ "bfabric-asgi-auth[demo]", - "pytest>=8.0.0", + "pytest>=8.0.0,<9.2", # shortcut: pytest 9.2 removed an internal pytest-bdd 9.0 uses; lift once pytest-bdd supports it "pytest-mock>=3.15.1", "pytest-asyncio>=1.2.0", "pytest-bdd>=8.1.0", diff --git a/bfabric_scripts/docs/changelog.md b/bfabric_scripts/docs/changelog.md index 65d49c092..062667c04 100644 --- a/bfabric_scripts/docs/changelog.md +++ b/bfabric_scripts/docs/changelog.md @@ -2,6 +2,7 @@ ### Fixed +- `bfabric-cli` reports user errors in one `Error:` line instead of a traceback: an unknown field (e.g. `api read ... --query status ...`), a missing file or config, a missing entity, a malformed config or JSON, an HTTP error from the OAuth endpoints, and an unknown method in `api inspect`. Set `BFABRICPY_LOG_LEVEL=DEBUG` to see the traceback. - `bfabric-cli login` and `auth device-code` no longer fail on an environment recorded with an `http://` base URL; they use `https://` and save the corrected URL. ### Changed diff --git a/bfabric_scripts/src/bfabric_scripts/cli/__main__.py b/bfabric_scripts/src/bfabric_scripts/cli/__main__.py index 42573f51f..17be4124b 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/__main__.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/__main__.py @@ -2,7 +2,7 @@ import cyclopts -from bfabric.utils.cli_integration import setup_script_logging +from bfabric.utils.cli_integration import report_user_errors, setup_script_logging from bfabric_scripts.cli.cli_api import cmd_api from bfabric_scripts.cli.cli_dataset import cmd_dataset from bfabric_scripts.cli.cli_executable import cmd_executable @@ -31,7 +31,8 @@ def main() -> None: """CLI entry point; sets logging up here because commands without ``@use_client`` would otherwise run at loguru's default DEBUG level.""" setup_script_logging() - app() + with report_user_errors(): + app() if __name__ == "__main__": diff --git a/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py b/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py index 3564f8e90..aed63cfa3 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py @@ -9,6 +9,7 @@ from typing import Any from pydantic import BaseModel, Field +from suds import MethodNotFound # pyright: ignore[reportMissingTypeStubs] from suds.xsd.query import TypeQuery # pyright: ignore[reportMissingTypeStubs] from bfabric import Bfabric @@ -45,7 +46,7 @@ def parse_method_signature( """Parses a SOAP method signature into reusable Pydantic models. :raises RuntimeError: If the client is not configured to use the SUDS engine. - :raises AttributeError: If the endpoint or method does not exist. + :raises ValueError: If the method does not exist on the endpoint. """ # The WSDL introspection below is written against SUDS internals, so reject any other engine up # front instead of leaking an AttributeError from the private engine. @@ -57,7 +58,10 @@ def parse_method_signature( service = client._engine._get_suds_service(endpoint) # type: ignore[attr-defined] # pyright: ignore[reportPrivateUsage,reportAttributeAccessIssue,reportUnknownVariableType,reportUnknownMemberType] - method = getattr(service, method_name) # pyright: ignore[reportAny,reportUnknownArgumentType] + try: + method = getattr(service, method_name) # pyright: ignore[reportAny,reportUnknownArgumentType] + except MethodNotFound as error: + raise ValueError(f"Unknown method {method_name!r} for the {endpoint!r} endpoint.") from error binding = method.method.binding.input # pyright: ignore[reportAny] param_defs = binding.param_defs(method.method) # pyright: ignore[reportAny] diff --git a/tests/bfabric/config/test_config_data.py b/tests/bfabric/config/test_config_data.py index e7e73010f..ef9f2d121 100644 --- a/tests/bfabric/config/test_config_data.py +++ b/tests/bfabric/config/test_config_data.py @@ -105,6 +105,12 @@ def test_from_file_and_file_does_not_exist(self, mocker): with pytest.raises(OSError, match="No explicit config provided, and no config file found at"): load_config_data(config_file_path="/dev/null", config_file_env="TEST", include_auth=True) + def test_from_file_unknown_env(self, mocker, example_config_path): + mock_env = mocker.patch.dict(os.environ) + mock_env.pop("BFABRICPY_CONFIG_OVERRIDE", None) + with pytest.raises(ValueError, match="Environment 'NOSUCHENV' not found in the config"): + load_config_data(config_file_path=example_config_path, config_file_env="NOSUCHENV", include_auth=True) + def test_export_roundtrip(config_data): exported = export_config_data(config_data) diff --git a/tests/bfabric/config/test_config_file.py b/tests/bfabric/config/test_config_file.py index d4541cd99..f980ae837 100644 --- a/tests/bfabric/config/test_config_file.py +++ b/tests/bfabric/config/test_config_file.py @@ -155,6 +155,12 @@ def test_get_selected_config(config_with_auth, mocker): mock_get_config_env.assert_called_once_with(explicit_config_env=None) +def test_get_selected_config_unknown_env(config_with_auth): + """A typo in ``--config-env`` names the available environments instead of raising a bare KeyError.""" + with pytest.raises(ValueError, match=r"Environment 'PRODUCTOIN' not found in the config; available: .*PRODUCTION"): + config_with_auth.get_selected_config("PRODUCTOIN") + + def test_reject_env_name_default(mocker, data_no_auth): data_no_auth["default"] = {"base_url": "https://example.com/bfabric"} with pytest.raises(ValueError) as error: diff --git a/tests/bfabric/engine/test_engine_suds.py b/tests/bfabric/engine/test_engine_suds.py index 2159bf2f8..52be05b30 100644 --- a/tests/bfabric/engine/test_engine_suds.py +++ b/tests/bfabric/engine/test_engine_suds.py @@ -3,7 +3,7 @@ import pytest import suds.transport from pydantic import SecretStr -from suds import MethodNotFound +from suds import MethodNotFound, TypeNotFound, WebFault from suds.client import Client from bfabric.engine.engine_suds import EngineSUDS @@ -67,7 +67,8 @@ def test_save(engine_suds, mock_auth, mock_suds_service, mocker): def test_save_method_not_found(engine_suds, mock_auth, mock_suds_service, mocker): - mock_suds_service.save.side_effect = MethodNotFound("save") + # suds raises it on the attribute lookup, not on the call + type(mock_suds_service).save = mocker.PropertyMock(side_effect=MethodNotFound("save")) with pytest.raises( BfabricRequestError, @@ -171,6 +172,46 @@ def test_missing_endpoint_still_reports_the_endpoint(self, mocker): assert not isinstance(exc_info.value, BfabricUnavailableError) +class TestServiceCallErrors: + """Errors raised by suds during a service call must arrive as a RuntimeError, not a traceback.""" + + @pytest.fixture(params=["read", "save", "delete"]) + def call(self, request, engine_suds, mock_auth, mock_suds_service): + method = request.param + args = {"read": {}, "save": {}, "delete": 1}[method] + return getattr(mock_suds_service, method), lambda: getattr(engine_suds, method)("sample", args, mock_auth) + + def test_unknown_field(self, call): + service_method, run = call + service_method.side_effect = TypeNotFound("status") + + with pytest.raises(BfabricRequestError) as exc_info: + run() + assert str(exc_info.value) == "Unknown field 'status' for the 'sample' endpoint." + + def test_server_fault(self, call, mocker): + service_method, run = call + service_method.side_effect = WebFault(mocker.MagicMock(faultstring="boom"), None) + + with pytest.raises(BfabricRequestError, match="boom"): + run() + + def test_http_error_status(self, call): + """suds reports a non-SOAP HTTP error status from a call as a bare ``Exception((status, reason))``.""" + service_method, run = call + service_method.side_effect = Exception((502, "Bad Gateway")) + + with pytest.raises(BfabricRequestError, match="502"): + run() + + def test_connection_failure(self, call): + service_method, run = call + service_method.side_effect = URLError(ConnectionResetError(104, "Connection reset")) + + with pytest.raises(BfabricUnavailableError): + run() + + def test_convert_results_no_results(engine_suds, mocker): mock_response = mocker.MagicMock() mock_response.numberofpages = 0 diff --git a/tests/bfabric/test_cpu_check.py b/tests/bfabric/test_cpu_check.py new file mode 100644 index 000000000..ea8554e3a --- /dev/null +++ b/tests/bfabric/test_cpu_check.py @@ -0,0 +1,11 @@ +import subprocess +import sys + + +def test_polars_missing_cpu_features_warning_raises(): + # pytest replaces warning filters per test, so check the real import in a fresh interpreter; + # the message mimics polars' _cpu_check, emitted right before the process dies with SIGILL + code = "import bfabric, warnings; warnings.warn('Missing required CPU features. Install polars[rtcompat]', RuntimeWarning)" + result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True) + assert result.returncode == 1 + assert "RuntimeWarning: Missing required CPU features" in result.stderr diff --git a/tests/bfabric/utils/test_cli_integration.py b/tests/bfabric/utils/test_cli_integration.py index 915ef3e46..92f343555 100644 --- a/tests/bfabric/utils/test_cli_integration.py +++ b/tests/bfabric/utils/test_cli_integration.py @@ -9,7 +9,9 @@ import typing from pathlib import Path +import httpx import pytest +import yaml from bfabric.utils import cli_integration from bfabric.utils.cli_integration import setup_script_logging, use_client @@ -326,6 +328,43 @@ def my_function(*, client: Bfabric) -> None: assert exc_info.value.code == 1 assert capsys.readouterr().err == "Error: no config\n" + @pytest.mark.parametrize( + "error", + [ + ValueError("no such dataset"), + FileNotFoundError("missing.csv"), + httpx.HTTPStatusError( + "400 Bad Request", request=httpx.Request("POST", "https://x"), response=httpx.Response(400) + ), + ], + ) + def test_user_error_from_the_command_is_reported_as_one_line(self, mocker, capsys, error): + mocker.patch("bfabric.Bfabric.connect", return_value=mocker.Mock()) + + @use_client + def my_function(*, client: Bfabric) -> None: + raise error + + with pytest.raises(SystemExit) as exc_info: + my_function() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == f"Error: {error}\n" + + @pytest.mark.parametrize("error", [OSError("no config file found"), yaml.YAMLError("bad yaml")]) + def test_unreadable_config_is_reported_as_one_line(self, mocker, capsys, error): + mocker.patch("bfabric.Bfabric.connect", side_effect=error) + + @use_client + def my_function(*, client: Bfabric) -> None: + raise AssertionError("must not be reached") + + with pytest.raises(SystemExit) as exc_info: + my_function() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == f"Error: {error}\n" + def test_traceback_is_kept_at_debug_level(self, mocker, mock_logger): """The traceback is still available for development via ``BFABRICPY_LOG_LEVEL=DEBUG``.""" mocker.patch("bfabric.Bfabric.connect", return_value=mocker.Mock()) diff --git a/tests/bfabric_cli/test_cli_dataset_download.py b/tests/bfabric_cli/test_cli_dataset_download.py index 635745e7d..3cc946210 100644 --- a/tests/bfabric_cli/test_cli_dataset_download.py +++ b/tests/bfabric_cli/test_cli_dataset_download.py @@ -138,19 +138,21 @@ def test_download_auto_format_excel(self, mocker, mock_client, mock_dataset, tmp mock_dataset.to_polars.return_value.write_excel.assert_called_once() - def test_download_auto_format_unknown_extension(self, mocker, mock_client, mock_dataset): + def test_download_auto_format_unknown_extension(self, mocker, mock_client, mock_dataset, capsys): mocker.patch.object(Dataset, "find", return_value=mock_dataset) params = Params(dataset_id=123, file=Path("output.txt"), format=OutputFormat.AUTO) - with pytest.raises(ValueError, match="Cannot infer format from file extension"): + with pytest.raises(SystemExit): cmd_dataset_download(params, client=mock_client) + assert "Cannot infer format from file extension" in capsys.readouterr().err - def test_download_dataset_not_found(self, mocker, mock_client): + def test_download_dataset_not_found(self, mocker, mock_client, capsys): mocker.patch.object(Dataset, "find", return_value=None) params = Params(dataset_id=999, file=Path("output.csv"), format=OutputFormat.CSV) - with pytest.raises(ValueError, match="Dataset with id 999 not found"): + with pytest.raises(SystemExit): cmd_dataset_download(params, client=mock_client) + assert capsys.readouterr().err == "Error: Dataset with id 999 not found.\n" def test_download_creates_directory(self, mocker, mock_client, mock_dataset, tmp_path): mocker.patch.object(Dataset, "find", return_value=mock_dataset) diff --git a/tests/bfabric_cli/test_cli_main.py b/tests/bfabric_cli/test_cli_main.py new file mode 100644 index 000000000..6dd3f27d4 --- /dev/null +++ b/tests/bfabric_cli/test_cli_main.py @@ -0,0 +1,15 @@ +import pytest + +from bfabric_scripts.cli import __main__ as cli_main + + +def test_main_reports_a_command_error_in_one_line(mocker, capsys): + """Commands without ``@use_client`` (e.g. ``auth``) share the same one-line error boundary.""" + mocker.patch.object(cli_main, "setup_script_logging") + mocker.patch.object(cli_main, "app", side_effect=ValueError("base URL must end in /bfabric")) + + with pytest.raises(SystemExit) as exc_info: + cli_main.main() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == "Error: base URL must end in /bfabric\n" diff --git a/tests/bfabric_scripts/test_api_parser.py b/tests/bfabric_scripts/test_api_parser.py index 976d5282f..77ebe9586 100644 --- a/tests/bfabric_scripts/test_api_parser.py +++ b/tests/bfabric_scripts/test_api_parser.py @@ -4,6 +4,7 @@ import pytest from pytest_mock import MockerFixture +from suds import MethodNotFound from bfabric.config.bfabric_client_config import BfabricAPIEngineType from bfabric_scripts.cli.api.parser import ( @@ -295,6 +296,17 @@ def test_parse_method_signature_rejects_non_suds_engine(self, mocker: MockerFixt # Guard must fire before any private engine access. client_mock._engine._get_suds_service.assert_not_called() + def test_parse_method_signature_unknown_method(self, mocker: MockerFixture) -> None: + """suds raises ``MethodNotFound`` on the lookup; it must arrive as a one-line ``ValueError``.""" + service_mock = mocker.MagicMock() + type(service_mock).raed = mocker.PropertyMock(side_effect=MethodNotFound("raed")) + client_mock = mocker.MagicMock() + client_mock.config.engine = BfabricAPIEngineType.SUDS + client_mock._engine._get_suds_service.return_value = service_mock + + with pytest.raises(ValueError, match="'raed'.*'resource'"): + parse_method_signature(client_mock, "resource", "raed") + if __name__ == "__main__": pytest.main([__file__, "-v"])