diff --git a/conftest.py b/conftest.py index cf73e19..59c82f6 100644 --- a/conftest.py +++ b/conftest.py @@ -1,21 +1,18 @@ """Pytest configuration for the http2py repository. -``http2py.api_pkg_maker`` imports ``setuptools.sandbox``, which modern -setuptools no longer ships, so merely *importing* the module raises -``ImportError``. Under ``--doctest-modules`` that is not a single failing test: -it aborts collection for the whole session. The module is deliberately left in -place (whether to rewrite it or remove it is still open on issue #14), so it is -excluded from collection instead. +``http2py/tests`` stands up a real ``py2http`` web service, so it depends on a +test-only package that is not one of http2py's declared dependencies. Under +``--doctest-modules`` an unimportable module is not a single failing test: it +aborts collection for the whole session. So that directory is excluded here. -CI excludes the same two paths via ``[tool.wads.ci.testing].exclude_paths`` in +CI excludes the same path via ``[tool.wads.ci.testing].exclude_paths`` in pyproject.toml, which the wads ``run-tests-uv`` action turns into ``--ignore`` -flags. Repeating them here is what makes a bare ``pytest`` (no flags, e.g. a -local run or an editor's test runner) behave the same way as CI. +flags. Repeating it here is what makes a bare ``pytest`` (no flags, e.g. a local +run or an editor's test runner) behave the same way as CI. ``tests/test_ci_collection_contract.py`` asserts the two lists stay in agreement, so they cannot drift apart silently. """ collect_ignore = [ - "http2py/api_pkg_maker.py", "http2py/tests", ] diff --git a/http2py/api_pkg_maker.py b/http2py/api_pkg_maker.py index d68e899..a61f26c 100644 --- a/http2py/api_pkg_maker.py +++ b/http2py/api_pkg_maker.py @@ -3,21 +3,32 @@ Given a spec (or the URL of one), :func:`mk_api_pkg` writes a small source distribution whose functions are bound to the service's routes. -Note: this module imports ``setuptools.sandbox``, which modern setuptools no -longer provides, so importing it raises ``ImportError``. It is kept in place -pending a decision (rewrite or remove) and is excluded from test collection; -see the repo-root ``conftest.py``. +The sdist is built by running ``setup.py sdist`` in a subprocess. This module +used to call ``setuptools.sandbox.run_setup``, which modern setuptools no longer +ships -- so merely *importing* it raised ``ImportError`` and the +``api-pkg-maker`` console script could not start at all (i2mint/http2py#14). +The subprocess is also better behaved: ``sandbox.run_setup`` needed the process +to ``os.chdir`` into the build directory and never came back. + +``setuptools`` is still needed to *run* :func:`mk_api_pkg`, but it is +deliberately NOT a declared dependency of http2py: it is needed by this one +function, not by ``import http2py``, and a virtualenv created by ``uv`` does not +ship it. :func:`_check_build_backend` turns its absence into an actionable +message instead of a subprocess exit code. """ -import argh import os import shutil +import subprocess +import sys import tempfile from http2py.client import HttpClient from datetime import datetime, timezone -from setuptools import sandbox -OUTPUT_DIR = os.path.join(os.environ["HOME"], "http2py", "api_pkgs") +#: Where built packages are written. ``expanduser`` rather than +#: ``os.environ["HOME"]``: the latter is a module-import-time ``KeyError`` on +#: Windows, which would take the whole package down with it. +OUTPUT_DIR = os.path.join(os.path.expanduser("~"), "http2py", "api_pkgs") INIT_FILE_TPL = """from .funcs import {funcs} """ @@ -111,6 +122,8 @@ def create_file(filepath, content): with open(filepath, "w") as file: print(content, file=file) + _check_build_backend() + tempdir = tempfile.mkdtemp() try: api = HttpClient(openapi_spec=openapi_spec, url=openapi_url) @@ -148,8 +161,20 @@ def create_file(filepath, content): filepath=os.path.join(tempdir, f"setup.cfg"), content=setup_cfg_content ) create_file(filepath=os.path.join(tempdir, f"setup.py"), content=SETUP_PY) - os.chdir(tempdir) - sandbox.run_setup("setup.py", ["sdist"]) + build = subprocess.run( + [sys.executable, "setup.py", "sdist"], + cwd=tempdir, + capture_output=True, + text=True, + ) + if build.returncode != 0: + # check=True would raise a CalledProcessError carrying only the exit + # status, discarding the captured output that says what went wrong. + raise RuntimeError( + f"`setup.py sdist` failed (exit {build.returncode}) while building " + f"{pkg_name!r}.\n--- stdout ---\n{build.stdout}" + f"\n--- stderr ---\n{build.stderr}" + ) pkg_filename = f"{pkg_name}-{pkg_version}.tar.gz" if not os.path.exists(OUTPUT_DIR): os.makedirs(OUTPUT_DIR) @@ -161,9 +186,34 @@ def create_file(filepath, content): shutil.rmtree(tempdir) +def _check_build_backend(): + """Fail early and legibly when the interpreter cannot build an sdist. + + ``setup.py sdist`` needs setuptools in the *subprocess's* interpreter, which + is this one. Environments created by ``uv`` (and ``python -m venv + --without-pip``) do not have it. Without this check the user sees a bare + ``CalledProcessError`` with an exit status and no cause. + """ + import importlib.util + + if importlib.util.find_spec("setuptools") is None: + raise RuntimeError( + "mk_api_pkg builds a source distribution with `setup.py sdist`, " + f"which needs setuptools in {sys.executable}. Install it with: " + f"{sys.executable} -m pip install setuptools" + ) + + def main(): - argh.dispatch_command(mk_api_pkg) + """Entry point for the ``api-pkg-maker`` console script; returns the exit code. + + ``cw.dispatch`` returns the code rather than exiting, and + ``[project.scripts]`` wraps this in ``sys.exit(main())``. + """ + import cw + + return cw.dispatch(mk_api_pkg) if __name__ == "__main__": - main() + raise SystemExit(main()) diff --git a/http2py/cli_maker.py b/http2py/cli_maker.py index de4c81a..15351b2 100644 --- a/http2py/cli_maker.py +++ b/http2py/cli_maker.py @@ -1,10 +1,14 @@ """Turn an http-bound python object into a command line interface. Signatures are first made argparse-friendly (:func:`mk_argparse_friendly`), -then dispatched with ``argh`` by :func:`mk_cli` / :func:`dispatch_cli`. +then turned into a parser by :func:`mk_cli` and run by :func:`dispatch_cli`. + +The parser is built by ``cw``. Its ``BY_NAME_IF_KWONLY`` naming is what +reproduces the grammar this module has always produced -- see +:data:`CLI_CONVENTION`. """ -import argh +import dataclasses from functools import wraps from glom import glom from inspect import signature @@ -15,11 +19,30 @@ from collections.abc import Callable, Iterable import yaml +import cw + from i2.io_trans import JSONAnnotAndDfltIoTrans from i2.signatures import set_signature_of_func, Sig, KO from http2py import HttpClient from http2py.authentication import mk_auth, DFLT_CONFIG_FILENAME +#: How the command line is derived from a signature. +#: +#: Every function :func:`register_cli_method` builds has KEYWORD-ONLY parameters +#: only (plus ``*args``/``**kwargs`` on a no-argument route), because +#: :func:`Sig.merge_with_sig` is called with ``kind=KO``. For that shape, +#: ``BY_NAME_IF_KWONLY`` is what reproduces the grammar this module produced +#: under argh: every parameter is an option, required exactly when it has no +#: default -- ``get-thing -u UID -p PID [-l LIMIT]``. +#: +#: cw's ``ARGH`` default (``BY_NAME_IF_HAS_DEFAULT``) would NOT: it turns a +#: keyword-only parameter with no default into a *positional*, so every required +#: API argument would silently change spelling. That is not a cw bug -- it +#: faithfully reproduces argh's *explicit* ``BY_NAME_IF_HAS_DEFAULT`` policy. +#: This module never selected a policy, and argh's no-policy legacy mode is the +#: one that matches ``BY_NAME_IF_KWONLY`` here. +CLI_CONVENTION = dataclasses.replace(cw.ARGH, naming=cw.BY_NAME_IF_KWONLY) + def mk_sig_argparse_friendly(sig): """Modifies a signature to change all leading underscores in param names @@ -109,15 +132,20 @@ def mk_cli( for methodname, method in client_details.__dict__.items() if getattr(method, "method_spec", None) ] - parser = argh.ArghParser() - parser.add_commands(cli_methods) - return parser + return cw.mk_parser(cli_methods, convention=CLI_CONVENTION) def dispatch_cli(*args, **kwargs): - """Makes a CLI parser with mk_cli and then dispatches it (see documentation of mk_cli)""" - parser = mk_cli(*args, **kwargs) - parser.dispatch() + """Makes a CLI parser with mk_cli and then runs it (see documentation of mk_cli) + + Returns ``None`` when the command ran, and raises ``SystemExit(2)`` on a + command-line error -- the same two outcomes the previous argh-based + implementation produced. ``cw.run`` *returns* the exit code rather than + exiting, so the non-zero case is re-raised here. + """ + code = cw.run(mk_cli(*args, **kwargs)) + if code: + raise SystemExit(code) set_signature_of_func(dispatch_cli, signature(mk_cli)) diff --git a/http2py/example_cli.py b/http2py/example_cli.py index 50a1403..fb36c7c 100644 --- a/http2py/example_cli.py +++ b/http2py/example_cli.py @@ -1,6 +1,6 @@ """A tiny worked example of the CLI-making tools, used by the docs and by hand.""" -import argh +import cw from collections.abc import Iterable from http2py.cli_maker import mk_argparse_friendly @@ -13,6 +13,11 @@ def myfun(a: int, b: Iterable[int]): if __name__ == "__main__": - parser = argh.ArghParser() - parser.add_commands([mk_argparse_friendly(AnnotAndDfltIoTrans()(myfun))]) - parser.dispatch() + from http2py.cli_maker import CLI_CONVENTION + + raise SystemExit( + cw.dispatch( + [mk_argparse_friendly(AnnotAndDfltIoTrans()(myfun))], + convention=CLI_CONVENTION, + ) + ) diff --git a/pyproject.toml b/pyproject.toml index 855e85a..63f63f3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ "i2", "ju", "requests", - "argh", + "cw>=0.1.1,<0.2", "PyYAML", "importlib_resources", # TEMPORARY, not imported by http2py: ju/oas.py has an unconditional (and @@ -57,6 +57,11 @@ dev = [ "pytest>=7.0", "pytest-cov>=4.0", "ruff>=0.1.0", + # Not a runtime dependency of the package: `mk_api_pkg` shells out to + # `setup.py sdist`, so setuptools is needed to exercise it. A uv-created + # venv does not ship setuptools, so without this the end-to-end + # api_pkg_maker test would silently skip in CI. + "setuptools", ] [tool.hatch.build.targets.wheel] @@ -160,15 +165,16 @@ coverage_threshold = 0 coverage_report_format = [ "term", ] -# `http2py.api_pkg_maker` imports `setuptools.sandbox`, which modern setuptools -# no longer ships, so the module raises ImportError on import and would abort -# the whole collection. Whether to rewrite it or drop it is still open on -# issue #14; excluding it from collection is the reversible middle course. -# The same two paths are repeated in the repo-root conftest.py so that a bare -# `pytest` behaves like CI -- tests/test_ci_collection_contract.py keeps the -# two lists in agreement. +# `http2py/tests` stands up a real py2http web service, so it needs a test-only +# dependency that CI does not install. Excluding it from collection is what keeps +# `pytest --doctest-modules` from aborting the session on the import. +# (`http2py/api_pkg_maker.py` used to be excluded here too, because +# `setuptools.sandbox` no longer exists and the module was unimportable. It is +# fixed and collected again -- see issue #14.) +# This path is repeated in the repo-root conftest.py so that a bare `pytest` +# behaves like CI -- tests/test_ci_collection_contract.py keeps the two lists +# in agreement. exclude_paths = [ - "http2py/api_pkg_maker.py", "http2py/tests", ] test_on_windows = false diff --git a/tests/cli_goldens/mk_cli.json b/tests/cli_goldens/mk_cli.json new file mode 100644 index 0000000..5636f57 --- /dev/null +++ b/tests/cli_goldens/mk_cli.json @@ -0,0 +1,10 @@ +{ + "recorded_from": "argh 0.31.3 via ArghParser().add_commands(...) with NO explicit name-mapping policy -- argh's legacy mode, which is what http2py.cli_maker.mk_cli used before the cw migration", + "note": "Normalised usage lines only: CPython rewrites --help bodies between versions and CI spans 3.10 and 3.12.", + "usages": { + "__top__": "usage: PROG [-h] {get-thing,make,plain} ...", + "get-thing": "usage: PROG get-thing [-h] -u UID -p PID [-v VERBOSE] [-l LIMIT] [-a API_KEY] [-c CONFIG]", + "make": "usage: PROG make [-h] -n NAME [--count COUNT] [-f FLAG] [-a API_KEY] [--config CONFIG]", + "plain": "usage: PROG plain [-h] [-a API_KEY] [-c CONFIG] [args ...]" + } +} \ No newline at end of file diff --git a/tests/spec_fixture.py b/tests/spec_fixture.py new file mode 100644 index 0000000..4fc3b75 --- /dev/null +++ b/tests/spec_fixture.py @@ -0,0 +1,89 @@ +"""One OpenAPI spec exercising every argument shape ``mk_cli`` has to handle. + +Path parameters (required, and typed), query parameters (optional, with and +without a schema default), a JSON request body with a ``required`` list, an +``apiKey`` security scheme (which adds an ``api_key`` CLI argument), and a route +with no arguments at all (which yields a ``*args``/``**kwargs`` signature). + +Kept as a module rather than inline so the golden recorder and the tests are +provably looking at the same spec. +""" + +CONTENT = "application/json" + +SPEC = { + "openapi": "3.0.2", + "info": {"title": "example", "version": "0.1"}, + "servers": [{"url": "https://example.com"}], + "security": {"apiKey": {}}, + "paths": { + "/u/{uid}/p/{pid}": { + "get": { + "x-method_name": "get_thing", + "description": "Two path args and two query args.", + "parameters": [ + { + "name": "uid", + "in": "path", + "required": True, + "schema": {"type": "string"}, + }, + { + "name": "pid", + "in": "path", + "required": True, + "schema": {"type": "integer"}, + }, + { + "name": "verbose", + "in": "query", + "required": False, + "schema": {"type": "boolean"}, + }, + { + "name": "limit", + "in": "query", + "required": False, + "schema": {"type": "integer", "default": 10}, + }, + ], + "responses": { + "200": {"description": "", "content": {CONTENT: {"schema": {}}}} + }, + } + }, + "/make": { + "post": { + "x-method_name": "make", + "description": "A request body with one required property.", + "requestBody": { + "content": { + CONTENT: { + "schema": { + "type": "object", + "properties": { + "name": {"type": "string"}, + "count": {"type": "integer"}, + "flag": {"type": "boolean"}, + }, + "required": ["name"], + } + } + } + }, + "responses": { + "200": {"description": "", "content": {CONTENT: {"schema": {}}}} + }, + } + }, + "/plain": { + "get": { + "x-method_name": "plain", + "description": "No arguments at all.", + "responses": { + "200": {"description": "", "content": {CONTENT: {"schema": {}}}} + }, + } + }, + }, +} diff --git a/tests/test_ci_collection_contract.py b/tests/test_ci_collection_contract.py index 105fd47..1094ac4 100644 --- a/tests/test_ci_collection_contract.py +++ b/tests/test_ci_collection_contract.py @@ -2,15 +2,16 @@ Under ``pytest --doctest-modules`` (what the CI test action runs) an unimportable module is not one red test, it is a collection abort that takes the whole session -down. Two modules in this repo are in that state: +down. -* ``http2py/api_pkg_maker.py`` imports ``setuptools.sandbox``, removed from - modern setuptools; -* ``http2py/tests/api_pkg_maker_test.py`` imports that module. +``http2py/api_pkg_maker.py`` used to be in that state -- it imported +``setuptools.sandbox``, removed from modern setuptools. It now builds the sdist +in a subprocess instead, imports cleanly, and is collected again. What remains +excluded is ``http2py/tests``, whose fixtures stand up a real ``py2http`` web +service: ``py2http`` is a test-only requirement that CI does not install. -Both are left in place on purpose (rewriting vs. deleting ``api_pkg_maker`` is -still open on issue #14) and excluded from collection in two places that must -agree: ``[tool.wads.ci.testing].exclude_paths`` in pyproject.toml, and +The exclusion lives in two places that must agree: +``[tool.wads.ci.testing].exclude_paths`` in pyproject.toml, and ``collect_ignore`` in the repo-root conftest.py. There is a third, subtler trap that these tests pin down: pytest eagerly imports @@ -31,10 +32,11 @@ REPO_ROOT = Path(__file__).resolve().parent.parent # Modules that are known-unimportable and therefore excluded from collection. -# Keep this in step with pyproject's exclude_paths and conftest's collect_ignore. -KNOWN_UNIMPORTABLE = {"http2py.api_pkg_maker"} +# Empty, and it should stay that way: an unimportable module aborts the whole +# CI session rather than failing one test. +KNOWN_UNIMPORTABLE: set = set() -EXPECTED_EXCLUDED_PATHS = {"http2py/api_pkg_maker.py", "http2py/tests"} +EXPECTED_EXCLUDED_PATHS = {"http2py/tests"} def _package_modules(): @@ -62,17 +64,27 @@ def test_every_package_module_imports_except_the_known_broken_one(): assert not failures, f"modules that would abort collection: {failures}" -def test_known_broken_module_is_still_broken(): - """Tripwire for issue #14. +def test_api_pkg_maker_imports_and_the_console_script_can_start(): + """The former tripwire for issue #14, inverted now that the module is fixed. - If ``api_pkg_maker`` starts importing again (someone rewrote it, or removed - it), the exclusions below are dead weight and the #14 (a)/(b) question is - answered -- so this deliberately fails to force that cleanup rather than - letting a stale exclusion sit there forever. + ``api-pkg-maker`` is this package's only console script, and it could not + start at all: ``from setuptools import sandbox`` raised ``ImportError`` at + import time, before ``main()`` was ever entered. Both halves are asserted -- + the module imports, and the entry point the console script calls is there. """ - for name in KNOWN_UNIMPORTABLE: - with pytest.raises(ImportError): - importlib.import_module(name) + module = importlib.import_module("http2py.api_pkg_maker") + assert callable(module.main) + assert callable(module.mk_api_pkg) + + +def test_no_module_is_excluded_for_being_unimportable(): + """Guard the invariant, not the past exception. + + ``KNOWN_UNIMPORTABLE`` is empty. If a future change adds a module that + cannot be imported, the honest fix is to fix the module -- not to grow this + set -- because an unimportable module aborts collection for everything. + """ + assert KNOWN_UNIMPORTABLE == set() @pytest.mark.skipif(sys.version_info < (3, 11), reason="tomllib needs Python 3.11+") @@ -137,3 +149,67 @@ def find_spec(self, fullname, path=None, target=None): sys.modules.pop("_http2py_tests_conftest", None) assert hasattr(module, "ws_app") + + +def test_mk_api_pkg_builds_a_source_distribution(tmp_path, monkeypatch): + """End-to-end proof that the ``setuptools.sandbox`` fix actually works. + + Builds a real sdist from a spec dict -- no network, no live service -- and + checks the archive is where ``mk_api_pkg`` says it is. Also asserts the + working directory survives: the old implementation did a bare + ``os.chdir(tempdir)`` and never came back, which quietly broke any caller + that used relative paths afterwards. + """ + import tarfile + + pytest.importorskip( + "setuptools", + reason="mk_api_pkg shells out to `setup.py sdist`; setuptools is in the " + "dev extra, so this runs in CI but skips in a bare environment", + ) + + from http2py import api_pkg_maker + + monkeypatch.setattr(api_pkg_maker, "OUTPUT_DIR", str(tmp_path)) + cwd_before = Path.cwd() + + spec = { + "openapi": "3.0.2", + "info": {"title": "d", "version": "0.1"}, + "servers": [{"url": "http://localhost:3030"}], + "paths": { + "/foo": { + "post": { + "x-method_name": "foo", + "description": "foo.", + "requestBody": { + "required": True, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": {"a": {"type": "integer"}}, + } + } + }, + }, + "responses": { + "200": { + "description": "", + "content": {"application/json": {"schema": {}}}, + } + }, + } + } + }, + } + + built = api_pkg_maker.mk_api_pkg(spec, pkg_name="probepkg", pkg_version="1.2.3") + + assert Path(built).is_file() + assert Path(built).name == "probepkg-1.2.3.tar.gz" + assert Path(built).parent == tmp_path + assert Path.cwd() == cwd_before, "mk_api_pkg left the process in another directory" + with tarfile.open(built) as archive: + names = {n.split("/", 1)[-1] for n in archive.getnames()} + assert "probepkg/funcs.py" in names diff --git a/tests/test_cli_surface.py b/tests/test_cli_surface.py new file mode 100644 index 0000000..f10c1b8 --- /dev/null +++ b/tests/test_cli_surface.py @@ -0,0 +1,171 @@ +"""Pin ``mk_cli``'s generated command line against what argh produced. + +``http2py.cli_maker`` builds a CLI out of an OpenAPI spec. The golden in +``tests/cli_goldens/mk_cli.json`` records the ``usage:`` line argh produced for +every command of ``tests/spec_fixture.SPEC``, recorded through +``ArghParser().add_commands(...)`` -- exactly the call ``mk_cli`` used to make -- +before the migration to ``cw``. + +Only usage lines are asserted, not full ``--help`` bodies: CPython rewrites the +option column between versions and CI spans 3.10 and 3.12. The full-body diff +(zero differences across all four surfaces) was done at migration time and is +recorded in the pull request. +""" + +from __future__ import annotations + +import dataclasses +import json +import sys +from pathlib import Path + +import pytest + +import cw + +sys.path.insert(0, str(Path(__file__).parent)) + +from spec_fixture import SPEC # noqa: E402 + +GOLDEN = json.loads( + (Path(__file__).parent / "cli_goldens" / "mk_cli.json").read_text(encoding="utf-8") +) +EXPECTED = GOLDEN["usages"] + + +def _usages(parser): + """``{command_name: normalised usage line}``, plus ``__top__`` for the parser.""" + out = {"__top__": " ".join(parser.format_usage().split())} + for action in parser._actions: + if getattr(action, "choices", None): + for name, sub in action.choices.items(): + out[name] = " ".join(sub.format_usage().split()) + return out + + +def _parser(**kwargs): + from http2py.cli_maker import mk_cli + + return mk_cli(openapi_spec=SPEC, **kwargs) + + +def test_mk_cli_returns_a_parser_with_one_command_per_route(): + parser = _parser() + assert set(_usages(parser)) == {"__top__", "get-thing", "make", "plain"} + + +@pytest.mark.parametrize("command", sorted(EXPECTED)) +def test_usage_matches_the_argh_recording(command): + """Every command's grammar is byte-identical to what argh generated. + + The recorded golden used ``prog="PROG"``; a parser built during a test run + derives its prog from ``sys.argv[0]``. Only what follows the prog is + compared, so both sides drop ``usage:`` plus the prog tokens. + """ + actual = _usages(_parser()) + offset = 1 if command == "__top__" else 2 # subparser progs are two tokens + assert actual[command].split()[1 + offset :] == EXPECTED[command].split()[ + 1 + offset : + ], f"{command}: {actual[command]!r} != {EXPECTED[command]!r}" + + +def test_required_api_arguments_are_options_not_positionals(): + """The load-bearing consequence of ``CLI_CONVENTION``. + + Every function ``register_cli_method`` builds is all-keyword-only, so a + required argument like ``uid`` must be spelled ``-u UID`` / ``--uid UID``. + Under cw's ARGH default it would become a bare positional and every caller's + command line would silently change. + """ + usage = _usages(_parser())["get-thing"] + assert "-u UID" in usage and "-p PID" in usage + assert " uid" not in usage and " pid" not in usage + + +def test_the_convention_is_load_bearing_and_this_test_can_fail(): + """Prove the check above would catch losing ``naming=BY_NAME_IF_KWONLY``. + + If this stops holding, cw's default naming has become equivalent for this + signature shape and ``CLI_CONVENTION``'s comment is stale -- not the other + way round. + """ + from http2py.cli_maker import CLI_CONVENTION, register_cli_method + from http2py.client import HttpClient + + client = HttpClient(SPEC) + methods = [ + register_cli_method(SPEC, m, ["api_key"]) + for _, m in client.__dict__.items() + if getattr(m, "method_spec", None) + ] + default = cw.mk_parser( + methods, + convention=dataclasses.replace(CLI_CONVENTION, naming=cw.ARGH.naming), + prog="PROG", + ) + usage = _usages(default)["get-thing"] + assert "-u UID" not in usage, ( + "cw's default naming was expected to turn the required keyword-only " + f"arguments into positionals, but produced {usage!r}" + ) + + +def test_every_parameter_register_cli_method_builds_is_keyword_only(): + """The premise ``CLI_CONVENTION`` rests on, asserted rather than assumed. + + ``BY_NAME_IF_KWONLY`` reproduces argh's legacy grammar *because* these + signatures are all keyword-only. If a future change introduced an ordinary + positional parameter, the two policies would diverge for it and the golden + above is what would catch it -- but this says so directly. + """ + import inspect + + from http2py.cli_maker import register_cli_method + from http2py.client import HttpClient + + client = HttpClient(SPEC) + for _, method in client.__dict__.items(): + if not getattr(method, "method_spec", None): + continue + func = register_cli_method(SPEC, method, ["api_key"]) + for param in inspect.signature(func).parameters.values(): + assert param.kind in ( + param.KEYWORD_ONLY, + param.VAR_POSITIONAL, + param.VAR_KEYWORD, + ), f"{func.__name__}.{param.name} is {param.kind}" + + +def test_dispatch_cli_raises_SystemExit_on_a_command_line_error(): + """``cw.run`` RETURNS the exit code; ``dispatch_cli`` must still raise it. + + argh's ``parser.dispatch()`` exited the process on a bad command line. + Without the re-raise, a broken invocation would look like success. + """ + from http2py.cli_maker import dispatch_cli + + argv = sys.argv + sys.argv = ["PROG", "no-such-command"] + try: + with pytest.raises(SystemExit) as excinfo: + dispatch_cli(openapi_spec=SPEC) + assert excinfo.value.code == 2 + finally: + sys.argv = argv + + +def test_the_package_no_longer_imports_argh(): + """http2py no longer declares argh; nothing may import it.""" + import subprocess + + proc = subprocess.run( + [ + sys.executable, + "-c", + "import sys, http2py, http2py.cli_maker, http2py.api_pkg_maker; " + "sys.exit(1 if 'argh' in sys.modules else 0)", + ], + capture_output=True, + text=True, + ) + assert proc.returncode == 0, f"argh was imported: {proc.stderr}"