From 1943777d79bba318a8223539c513a413d4876673 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 27 Sep 2026 22:49:07 +0300 Subject: [PATCH 1/2] ci: fetch the ADR citation check from .github instead of keeping a copy (#144) * ci: fetch the ADR citation check from .github instead of keeping a copy * ci: fetch the ADR check into the repo and run it without the repo's conftests or addopts --- .github/workflows/_checks.yml | 1 + justfile | 11 ++++ tests/test_adr_citations.py | 111 ---------------------------------- 3 files changed, 12 insertions(+), 111 deletions(-) delete mode 100644 tests/test_adr_citations.py diff --git a/.github/workflows/_checks.yml b/.github/workflows/_checks.yml index d63e9ff..0a4305a 100644 --- a/.github/workflows/_checks.yml +++ b/.github/workflows/_checks.yml @@ -15,6 +15,7 @@ jobs: - run: uv python install 3.11 - run: uv python pin 3.11 - run: just install lint-ci + - run: just adr-check docs: runs-on: ubuntu-latest diff --git a/justfile b/justfile index eb6ef12..e84727d 100644 --- a/justfile +++ b/justfile @@ -16,6 +16,17 @@ lint-ci: uv run ruff check --no-fix uv run ty check +adr_check_source := "https://raw.githubusercontent.com/modern-python/.github/main/tests/test_adr_citations.py" + +# Tracks main on purpose: the shared check is unpinned. +adr-check: + #!/usr/bin/env sh + set -eu + dir="$(mktemp -d .adr-check.XXXXXX)" + trap 'rm -rf "$dir"' EXIT + curl -fsSL "{{ adr_check_source }}" -o "$dir/test_adr_citations.py" + uv run --no-sync pytest --rootdir=. --noconftest -o addopts= "$dir/test_adr_citations.py" + test *args: uv run --no-sync pytest {{ args }} diff --git a/tests/test_adr_citations.py b/tests/test_adr_citations.py deleted file mode 100644 index 7f7706c..0000000 --- a/tests/test_adr_citations.py +++ /dev/null @@ -1,111 +0,0 @@ -import ast -import os -import pathlib -import re -import typing - - -_REPO_ROOT: typing.Final = pathlib.Path(__file__).resolve().parent.parent -_ADR_DIR: typing.Final = "docs/adr/" -_CITATION: typing.Final = re.compile(r"docs/adr/\d{4}(?:-[a-z0-9-]+\.md)?") -_UNWALKED_DIR: typing.Final = "node_modules" - - -def _python_files(root: pathlib.Path) -> list[pathlib.Path]: - found: list[pathlib.Path] = [] - for dirpath, dirnames, filenames in os.walk(root): - dirnames[:] = sorted(name for name in dirnames if not name.startswith(".") and name != _UNWALKED_DIR) - found.extend(pathlib.Path(dirpath, name) for name in sorted(filenames) if name.endswith(".py")) - return found - - -def _citations(source: str) -> set[str]: - texts = [source] - texts.extend( - node.value - for node in ast.walk(ast.parse(source)) - if isinstance(node, ast.Constant) and isinstance(node.value, str) - ) - return {cited for text in texts for cited in _CITATION.findall(text)} - - -def unresolved_citations(root: pathlib.Path) -> list[tuple[str, str]]: - return sorted( - (file.relative_to(root).as_posix(), cited) - for file in _python_files(root) - for cited in _citations(file.read_text(encoding="utf-8")) - if not (root / cited).is_file() - ) - - -def test_every_adr_path_cited_from_python_resolves() -> None: - """INVARIANT: a `docs/adr/NNNN-.md` path named anywhere in this repo's Python exists. - - Broken by renaming, renumbering or pruning an ADR without following its citations. The - offline link gate reads Markdown only, so a path in a docstring, a comment or a guard message - is otherwise checked by nothing, and an `INVARIANT:` docstring that names its ADR silently - loses the rationale the test depends on. A user who trips a guard is handed a link to follow. - A bare `docs/adr/NNNN` is reported as well: it names no file, so it would survive the same - rename or drop unnoticed and point at whatever record holds that number next. - """ - unresolved = unresolved_citations(_REPO_ROOT) - - assert unresolved == [], "\n".join(f"{file} cites {cited}" for file, cited in unresolved) - - -def test_a_citation_of_a_missing_adr_is_reported_with_its_citing_file(tmp_path: pathlib.Path) -> None: - """The scanner is exercised against a known result, so an empty scan cannot pass as a green one.""" - (tmp_path / _ADR_DIR).mkdir(parents=True) - (tmp_path / _ADR_DIR / "0001-kept.md").write_text("# kept\n", encoding="utf-8") - (tmp_path / "pkg").mkdir() - (tmp_path / "pkg" / "mod.py").write_text( - f'"""Explained in {_ADR_DIR}0001-kept.md and {_ADR_DIR}9999-missing.md."""\n', - encoding="utf-8", - ) - - assert unresolved_citations(tmp_path) == [("pkg/mod.py", f"{_ADR_DIR}9999-missing.md")] - - -def test_a_short_form_citation_is_reported_even_when_the_adr_exists(tmp_path: pathlib.Path) -> None: - """`docs/adr/NNNN` with no slug names nothing on disk, so a rename or a drop never breaks it.""" - (tmp_path / _ADR_DIR).mkdir(parents=True) - (tmp_path / _ADR_DIR / "0002-kept.md").write_text("# kept\n", encoding="utf-8") - (tmp_path / "pkg").mkdir() - (tmp_path / "pkg" / "mod.py").write_text( - f'"""Argued in {_ADR_DIR}0002 and {_ADR_DIR}0002-kept.md."""\n', encoding="utf-8" - ) - - assert unresolved_citations(tmp_path) == [("pkg/mod.py", f"{_ADR_DIR}0002")] - - -def test_a_citation_split_across_adjacent_string_literals_is_found(tmp_path: pathlib.Path) -> None: - """Python joins adjacent literals at parse time, which is what the `nack` guard message relies on.""" - (tmp_path / "guard.py").write_text( - f'MESSAGE = (\n "See https://example.invalid/blob/main/{_ADR_DIR}"\n "0003-split.md."\n)\n', - encoding="utf-8", - ) - - assert unresolved_citations(tmp_path) == [("guard.py", f"{_ADR_DIR}0003-split.md")] - - -def test_a_citation_inside_a_hash_comment_is_found(tmp_path: pathlib.Path) -> None: - """Comments never reach the AST, so the raw text is scanned as well.""" - (tmp_path / "graph.py").write_text(f"# The rule is one-way, see {_ADR_DIR}0009-comment.md\n", encoding="utf-8") - - assert unresolved_citations(tmp_path) == [("graph.py", f"{_ADR_DIR}0009-comment.md")] - - -def test_a_tree_with_no_citations_and_no_adr_directory_reports_nothing(tmp_path: pathlib.Path) -> None: - (tmp_path / "plain.py").write_text("X = 1\n", encoding="utf-8") - - assert unresolved_citations(tmp_path) == [] - - -def test_files_under_dot_directories_are_not_scanned(tmp_path: pathlib.Path) -> None: - """A virtualenv or a cache is not this repo's citations.""" - (tmp_path / ".venv" / "lib").mkdir(parents=True) - (tmp_path / ".venv" / "lib" / "vendored.py").write_text(f"# {_ADR_DIR}0001-elsewhere.md\n", encoding="utf-8") - (tmp_path / _UNWALKED_DIR).mkdir() - (tmp_path / _UNWALKED_DIR / "dep.py").write_text(f"# {_ADR_DIR}0002-elsewhere.md\n", encoding="utf-8") - - assert unresolved_citations(tmp_path) == [] From 8878a3dd7764f7f01eaff127869f7a1eb755e565 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Mon, 28 Sep 2026 23:16:24 +0300 Subject: [PATCH 2/2] fix: keep the URL's query string when params are passed (#145) httpx2 replaces a URL's query whenever params are sent, including client-level params (pydantic/httpx2#905). Fold the URL query into params in build_request, the per-verb methods and stream, for both clients. Precedence: client params < URL query < per-request params. --- src/httpware/client.py | 32 +++++++--- tests/test_url_query_merge.py | 107 ++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 tests/test_url_query_merge.py diff --git a/src/httpware/client.py b/src/httpware/client.py index 0288e44..760eec3 100644 --- a/src/httpware/client.py +++ b/src/httpware/client.py @@ -145,6 +145,16 @@ def _assemble_request_kwargs( # noqa: PLR0913 — 9 per-request kwargs from htt return kwargs +def _merge_url_query( + url: httpx2.URL | str, params: typing.Any | None, client_params: httpx2.QueryParams +) -> tuple[httpx2.URL | str, typing.Any | None]: + """Fold the URL's own query into `params`; httpx2 would otherwise replace it.""" + parsed = httpx2.URL(url) + if not parsed.query or (params is None and not client_params): + return url, params + return parsed.copy_with(query=None), parsed.params.merge(params) + + class AsyncClient: """Async HTTP client: thin wrapper around httpx2 with typed decoding and middleware.""" @@ -261,8 +271,9 @@ async def send_with_response( return response, bound.decode(response) def build_request(self, method: str, url: str, **kwargs: typing.Any) -> httpx2.Request: - """Delegate request construction to the wrapped httpx2.AsyncClient.""" - return self._httpx2_client.build_request(method, url, **kwargs) + """Delegate request construction to the wrapped httpx2.AsyncClient, keeping the URL's own query.""" + merged_url, params = _merge_url_query(url, kwargs.pop("params", None), self._httpx2_client.params) + return self._httpx2_client.build_request(method, merged_url, params=params, **kwargs) def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwargs-forwarding complexity is structural self, @@ -279,6 +290,7 @@ def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; data: typing.Any | None = None, files: typing.Any | None = None, ) -> httpx2.Request: + merged_url, params = _merge_url_query(url, params, self._httpx2_client.params) kwargs = _assemble_request_kwargs( params=params, headers=headers, @@ -290,7 +302,7 @@ def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; data=data, files=files, ) - request = self._httpx2_client.build_request(method, url, **kwargs) + request = self._httpx2_client.build_request(method, merged_url, **kwargs) if _is_streaming_body_async(content) or _is_streaming_body_async(data) or _is_streaming_body_async(files): request.extensions[STREAMING_BODY_MARKER] = True return request @@ -1046,6 +1058,7 @@ async def stream( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwa Maps httpx2 exceptions raised during the request OR body consumption to httpware exceptions via _httpx2_exception_mapper. """ + merged_url, params = _merge_url_query(url, params, self._httpx2_client.params) kwargs = _assemble_request_kwargs( params=params, headers=headers, @@ -1058,7 +1071,7 @@ async def stream( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwa files=files, ) - async with _httpx2_exception_mapper(), self._httpx2_client.stream(method, url, **kwargs) as response: + async with _httpx2_exception_mapper(), self._httpx2_client.stream(method, merged_url, **kwargs) as response: if HTTPStatus.BAD_REQUEST <= response.status_code < 600: # noqa: PLR2004 — 600 is the synthetic upper bound for 5xx cap = self._max_response_body_bytes if cap is None: @@ -1234,8 +1247,9 @@ def send_with_response( return response, bound.decode(response) def build_request(self, method: str, url: str, **kwargs: typing.Any) -> httpx2.Request: - """Delegate request construction to the wrapped httpx2.Client.""" - return self._httpx2_client.build_request(method, url, **kwargs) + """Delegate request construction to the wrapped httpx2.Client, keeping the URL's own query.""" + merged_url, params = _merge_url_query(url, kwargs.pop("params", None), self._httpx2_client.params) + return self._httpx2_client.build_request(method, merged_url, params=params, **kwargs) def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwargs-forwarding complexity is structural self, @@ -1252,6 +1266,7 @@ def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; data: typing.Any | None = None, files: typing.Any | None = None, ) -> httpx2.Request: + merged_url, params = _merge_url_query(url, params, self._httpx2_client.params) kwargs = _assemble_request_kwargs( params=params, headers=headers, @@ -1263,7 +1278,7 @@ def _prepare_request( # noqa: PLR0913 — mirrors httpx2 per-method signatures; data=data, files=files, ) - request = self._httpx2_client.build_request(method, url, **kwargs) + request = self._httpx2_client.build_request(method, merged_url, **kwargs) if _is_streaming_body_sync(content) or _is_streaming_body_sync(data) or _is_streaming_body_sync(files): request.extensions[STREAMING_BODY_MARKER] = True return request @@ -2016,6 +2031,7 @@ def stream( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwargs-fo Maps httpx2 exceptions raised during the request OR body consumption to httpware exceptions via _httpx2_exception_mapper_sync. """ + merged_url, params = _merge_url_query(url, params, self._httpx2_client.params) kwargs = _assemble_request_kwargs( params=params, headers=headers, @@ -2028,7 +2044,7 @@ def stream( # noqa: PLR0913 — mirrors httpx2 per-method signatures; kwargs-fo files=files, ) - with _httpx2_exception_mapper_sync(), self._httpx2_client.stream(method, url, **kwargs) as response: + with _httpx2_exception_mapper_sync(), self._httpx2_client.stream(method, merged_url, **kwargs) as response: if HTTPStatus.BAD_REQUEST <= response.status_code < 600: # noqa: PLR2004 — 600 is the synthetic upper bound for 5xx cap = self._max_response_body_bytes if cap is None: diff --git a/tests/test_url_query_merge.py b/tests/test_url_query_merge.py new file mode 100644 index 0000000..5edce20 --- /dev/null +++ b/tests/test_url_query_merge.py @@ -0,0 +1,107 @@ +"""The URL's own query string survives per-request and client-level `params`.""" + +from http import HTTPStatus + +import httpx2 +import pytest + +from httpware import AsyncClient, Client + + +def _ok(request: httpx2.Request) -> httpx2.Response: + return httpx2.Response(HTTPStatus.OK, request=request) + + +def _async_client(captured: list[httpx2.Request], params: dict[str, str] | None = None) -> AsyncClient: + def handler(request: httpx2.Request) -> httpx2.Response: + captured.append(request) + return _ok(request) + + return AsyncClient(httpx2_client=httpx2.AsyncClient(transport=httpx2.MockTransport(handler), params=params)) + + +def _sync_client(captured: list[httpx2.Request], params: dict[str, str] | None = None) -> Client: + def handler(request: httpx2.Request) -> httpx2.Response: + captured.append(request) + return _ok(request) + + return Client(httpx2_client=httpx2.Client(transport=httpx2.MockTransport(handler), params=params)) + + +_CASES = [ + pytest.param("https://example.test/x?a=1", {"b": "2"}, {}, "a=1&b=2", id="url-query-plus-params"), + pytest.param("https://example.test/x?a=1&b=1", {"b": "2"}, {}, "a=1&b=2", id="params-override-same-key"), + pytest.param("https://example.test/x?a=1&a=2", {"b": "3"}, {}, "a=1&a=2&b=3", id="repeated-url-keys-kept"), + pytest.param("https://example.test/x?a=1", {}, {}, "a=1", id="empty-params-keeps-url-query"), + pytest.param("https://example.test/x?a=1", None, {"c": "3"}, "c=3&a=1", id="client-params-keep-url-query"), + pytest.param( + "https://example.test/x?a=1&c=1", {"b": "2"}, {"c": "3"}, "c=1&a=1&b=2", id="url-query-overrides-client" + ), + pytest.param("https://example.test/x", {"b": "2"}, {"c": "3"}, "c=3&b=2", id="no-url-query-unchanged"), +] + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +def test_async_build_request_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + client = _async_client([], params=client_params) + assert client.build_request("GET", url, params=params).url.query == expected_query.encode() + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +def test_sync_build_request_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + client = _sync_client([], params=client_params) + assert client.build_request("GET", url, params=params).url.query == expected_query.encode() + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +async def test_async_get_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + captured: list[httpx2.Request] = [] + await _async_client(captured, params=client_params).get(url, params=params) + assert captured[0].url.query == expected_query.encode() + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +def test_sync_get_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + captured: list[httpx2.Request] = [] + _sync_client(captured, params=client_params).get(url, params=params) + assert captured[0].url.query == expected_query.encode() + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +async def test_async_stream_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + captured: list[httpx2.Request] = [] + async with _async_client(captured, params=client_params).stream("GET", url, params=params): + pass + assert captured[0].url.query == expected_query.encode() + + +@pytest.mark.parametrize(("url", "params", "client_params", "expected_query"), _CASES) +def test_sync_stream_merges_url_query( + url: str, params: dict[str, str] | None, client_params: dict[str, str], expected_query: str +) -> None: + captured: list[httpx2.Request] = [] + with _sync_client(captured, params=client_params).stream("GET", url, params=params): + pass + assert captured[0].url.query == expected_query.encode() + + +def test_owned_client_relative_url_merges_with_base_url() -> None: + client = Client(base_url="https://example.test/api", params={"c": "3"}) + request = client.build_request("GET", "items?a=1", params={"b": "2"}) + assert str(request.url) == "https://example.test/api/items?c=3&a=1&b=2" + + +def test_url_query_without_params_is_left_verbatim() -> None: + client = _sync_client([]) + url = "https://example.test/x?cursor=a%2Fb+c" + assert str(client.build_request("GET", url).url) == url