diff --git a/ddev/changelog.d/24289.added b/ddev/changelog.d/24289.added new file mode 100644 index 0000000000000..7bbf7b5faabe1 --- /dev/null +++ b/ddev/changelog.d/24289.added @@ -0,0 +1 @@ +Add support for the `--env` option to `ddev env agent` to pass environment variables. diff --git a/ddev/src/ddev/cli/env/agent.py b/ddev/src/ddev/cli/env/agent.py index 248a431d9de0f..d67192f00e03e 100644 --- a/ddev/src/ddev/cli/env/agent.py +++ b/ddev/src/ddev/cli/env/agent.py @@ -12,14 +12,21 @@ from ddev.e2e.agent.interface import AgentInterface -def _invoke_check_with_retry(agent: AgentInterface, args: list[str], *, retries: int = 3, backoff: float = 0.5) -> None: +def _invoke_check_with_retry( + agent: AgentInterface, + args: list[str], + *, + env_vars: dict[str, str] | None = None, + retries: int = 3, + backoff: float = 0.5, +) -> None: """Invoke ``agent check`` with bounded retry to absorb transient autodiscovery-reload races.""" import subprocess import time for attempt in range(retries + 1): try: - agent.invoke(args) + agent.invoke(args, env_vars=env_vars) return except subprocess.CalledProcessError: if attempt >= retries: @@ -31,6 +38,17 @@ def _invoke_check_with_retry(agent: AgentInterface, args: list[str], *, retries: time.sleep(backoff) +def _validate_env_vars(ctx: click.Context, param: click.Parameter, value: tuple[str, ...]) -> dict[str, str] | None: + env_vars: dict[str, str] = {} + for entry in value: + key, sep, env_value = entry.partition('=') + if not sep or not key: + raise click.BadParameter(f'`{entry}` is not in KEY=VALUE format', ctx=ctx, param=param) + env_vars[key] = env_value + + return env_vars or None + + @click.command( short_help='Invoke the Agent', context_settings={'help_option_names': [], 'ignore_unknown_options': True} ) @@ -38,8 +56,24 @@ def _invoke_check_with_retry(agent: AgentInterface, args: list[str], *, retries: @click.argument('environment') @click.argument('args', required=True, nargs=-1) @click.option('--config-file', hidden=True) +@click.option( + '--env', + 'env_vars', + multiple=True, + metavar='KEY=VALUE', + callback=_validate_env_vars, + help='Set an environment variable for this invocation only (may be repeated)', +) @click.pass_obj -def agent(app: Application, *, intg_name: str, environment: str, args: tuple[str, ...], config_file: str | None): +def agent( + app: Application, + *, + intg_name: str, + environment: str, + args: tuple[str, ...], + config_file: str | None, + env_vars: dict[str, str] | None, +): """ Invoke the Agent. """ @@ -75,11 +109,13 @@ def agent(app: Application, *, intg_name: str, environment: str, args: tuple[str if config_file is None or not trigger_run: try: if trigger_run: - _invoke_check_with_retry(agent, full_args) + _invoke_check_with_retry(agent, full_args, env_vars=env_vars) else: - agent.invoke(full_args) + agent.invoke(full_args, env_vars=env_vars) except subprocess.CalledProcessError as e: app.abort(code=e.returncode) + except NotImplementedError as e: + app.abort(str(e)) return @@ -90,7 +126,9 @@ def agent(app: Application, *, intg_name: str, environment: str, args: tuple[str if not env_data.config_file.is_file(): try: env_data.write_config(config) - _invoke_check_with_retry(agent, full_args) + _invoke_check_with_retry(agent, full_args, env_vars=env_vars) + except NotImplementedError as e: + app.abort(str(e)) finally: env_data.config_file.unlink() else: @@ -98,6 +136,8 @@ def agent(app: Application, *, intg_name: str, environment: str, args: tuple[str env_data.config_file.replace(temp_config_file) try: env_data.write_config(config) - _invoke_check_with_retry(agent, full_args) + _invoke_check_with_retry(agent, full_args, env_vars=env_vars) + except NotImplementedError as e: + app.abort(str(e)) finally: temp_config_file.replace(env_data.config_file) diff --git a/ddev/src/ddev/e2e/agent/docker.py b/ddev/src/ddev/e2e/agent/docker.py index 5ba3b61969837..274cf07518191 100644 --- a/ddev/src/ddev/e2e/agent/docker.py +++ b/ddev/src/ddev/e2e/agent/docker.py @@ -121,10 +121,12 @@ def _python_path(self) -> str: else f'/opt/datadog-agent/embedded/bin/python{self.python_version[0]}' ) - def _format_command(self, command: list[str]) -> list[str]: + def _format_command(self, command: list[str], *, env_vars: dict[str, str] | None = None) -> list[str]: cmd = ['docker', 'exec'] if self._isatty: cmd.append('-it') + for key, value in (env_vars or {}).items(): + cmd.extend(['-e', f'{key}={value}']) cmd.append(self._container_name) if command[0] == 'pip': @@ -364,11 +366,11 @@ def restart(self) -> None: f'Unable to restart Agent container `{self._container_name}`: {process.stdout.decode("utf-8")}' ) - def invoke(self, args: list[str]) -> None: - self.run_command(['agent', *args]) + def invoke(self, args: list[str], *, env_vars: dict[str, str] | None = None) -> None: + self.run_command(['agent', *args], env_vars=env_vars) - def run_command(self, args: list[str]) -> None: - self._run_command(self._format_command([*args]), check=True) + def run_command(self, args: list[str], *, env_vars: dict[str, str] | None = None) -> None: + self._run_command(self._format_command([*args], env_vars=env_vars), check=True) def enter_shell(self) -> None: self._run_command(self._format_command(['cmd' if self._is_windows_container else 'bash']), check=True) diff --git a/ddev/src/ddev/e2e/agent/interface.py b/ddev/src/ddev/e2e/agent/interface.py index fd05b50828413..135d832a5e6d6 100644 --- a/ddev/src/ddev/e2e/agent/interface.py +++ b/ddev/src/ddev/e2e/agent/interface.py @@ -73,7 +73,7 @@ def stop(self) -> None: ... def restart(self) -> None: ... @abstractmethod - def invoke(self, args: list[str]) -> None: ... + def invoke(self, args: list[str], *, env_vars: dict[str, str] | None = None) -> None: ... @abstractmethod def enter_shell(self) -> None: ... diff --git a/ddev/src/ddev/e2e/agent/vagrant.py b/ddev/src/ddev/e2e/agent/vagrant.py index d51c61ecb4507..f79e7e1a911dd 100644 --- a/ddev/src/ddev/e2e/agent/vagrant.py +++ b/ddev/src/ddev/e2e/agent/vagrant.py @@ -116,7 +116,10 @@ def restart(self) -> None: self._run_commands(guest_cmds, "restart_agent_service") self.app.display_info("Datadog Agent service restart sequence completed.") - def invoke(self, args: list[str]) -> None: + def invoke(self, args: list[str], *, env_vars: dict[str, str] | None = None) -> None: + if env_vars: + raise NotImplementedError("Per-invocation env_vars are not supported for the Vagrant agent") + agent_bin = LINUX_AGENT_BIN_PATH if not self._is_windows_vm else WINDOWS_AGENT_BIN_PATH guest_cmd_parts = ["sudo", agent_bin] + args if not self._is_windows_vm else [agent_bin] + args host_cmd = self._format_command(guest_cmd_parts) diff --git a/ddev/tests/AGENTS.md b/ddev/tests/AGENTS.md new file mode 100644 index 0000000000000..03d48ad6c3e59 --- /dev/null +++ b/ddev/tests/AGENTS.md @@ -0,0 +1,70 @@ +# ddev Test Guidelines + +These conventions apply to the tests under `ddev/tests/`. They supplement the repository root +`AGENTS.md`; where they overlap, the root file still applies. + +## Importing from `conftest.py` + +**Runtime rule: nothing imports from a `conftest.py` at runtime.** Pytest loads +conftest files through its own plugin machinery; importing one as a regular +module can create a second copy, double-registering fixtures and duplicating +module state. It may work in today's layout; it breaks under +`--import-mode=importlib`. + +**The one exception: fixture types.** A type that describes what a fixture +returns (e.g. a Protocol for a factory fixture's callable) lives next to that +fixture in `conftest.py`, so the fixture itself is typed where it is defined. +Tests that need it for annotations import it under `if TYPE_CHECKING:` only: + + # conftest.py + class ClientFactory(Protocol): + def __call__(self, transport: httpx.MockTransport) -> AsyncGitHubClient: ... + + @pytest.fixture + def client_factory(...) -> ClientFactory: ... + + # test_x.py + if TYPE_CHECKING: + from .conftest import ClientFactory + + def test_y(client_factory: ClientFactory) -> None: ... + +The guard is what makes this safe: the import never runs, so no second module +is created. A conftest import outside `if TYPE_CHECKING:` is a bug, and a +`TYPE_CHECKING` import of anything other than types (helpers, constants, +payload factories) is too — needing one of those at runtime means it belongs +in a helpers module. + +## Shared test code: scopes and placement + +Helpers live at the narrowest scope that contains all their users. Three scopes +exist, from narrowest to broadest: + +| Scope | Location | Who may use it | +|-----------|--------------------------------------------|------------------------------------------| +| Local | `helpers.py` next to the tests | Tests in that one package | +| Subtree | `helpers/` package at a subtree root (e.g. `tests/x/mypackage/helpers/`) | Test packages anywhere under that subtree | +| Global | `tests/helpers/` | Any test suite | + +Module vs package is size, not meaning: a `helpers.py` that outgrows one file +becomes a `helpers/` package with the public names re-exported from +`__init__.py`, and its scope does not change by doing so. + +**Placement rules:** + +- A new helper starts local. When a second package under the same subtree needs + it, promote it to the subtree `helpers/`. When packages outside the subtree + need it, promote it to `tests/helpers/`. Promote, never copy. +- Imports point upward only: tests and local helpers may import from their + subtree helpers and from `tests/helpers/`; subtree helpers may import from + `tests/helpers/`. Never import sideways from another package's local + `helpers.py` — a sideways import means the helper's scope is wrong; promote it. +- `conftest.py` at any level may import from helpers at its level or above. + Helpers never import from any conftest (see the conftest section). +- Helpers contain no fixtures. Anything decorated with `@pytest.fixture` + belongs in a `conftest.py`; helpers hold the plain functions, factories, + transports, registries, and types those fixtures and tests build on. + +Litmus test when adding a helper: list who uses it today, find their nearest +common ancestor in the tree, and put it there. If you cannot name a second +user, it is local. diff --git a/ddev/tests/cli/env/test_agent.py b/ddev/tests/cli/env/test_agent.py index 92d951f0a6914..73d911e4c994b 100644 --- a/ddev/tests/cli/env/test_agent.py +++ b/ddev/tests/cli/env/test_agent.py @@ -1,6 +1,8 @@ # (C) Datadog, Inc. 2023-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +import pytest + from ddev.e2e.config import EnvDataStorage @@ -35,7 +37,108 @@ def test_not_trigger_run(ddev, data_dir, mocker): assert result.exit_code == 0, result.output assert not result.output - invoke.assert_called_once_with(['status']) + invoke.assert_called_once_with(['status'], env_vars=None) + + +def test_env_vars(ddev, data_dir, mocker): + invoke = mocker.patch('ddev.e2e.agent.docker.DockerAgent.invoke') + + integration = 'postgres' + environment = 'py3.12' + env_data = EnvDataStorage(data_dir).get(integration, environment) + env_data.write_metadata({}) + + result = ddev('env', 'agent', integration, environment, '--env', 'FOO=bar', '--env', 'BAZ=qux', 'status') + + assert result.exit_code == 0, result.output + assert not result.output + + invoke.assert_called_once_with(['status'], env_vars={'FOO': 'bar', 'BAZ': 'qux'}) + + +def test_env_vars_trigger_run(ddev, data_dir, mocker): + invoke = mocker.patch('ddev.e2e.agent.docker.DockerAgent.invoke') + + integration = 'postgres' + environment = 'py3.12' + env_data = EnvDataStorage(data_dir).get(integration, environment) + env_data.write_metadata({}) + + result = ddev('env', 'agent', integration, environment, '--env', 'FOO=bar', 'check', integration, '-l', 'debug') + + assert result.exit_code == 0, result.output + assert not result.output + + invoke.assert_called_once_with(['check', integration, '-l', 'debug'], env_vars={'FOO': 'bar'}) + + +@pytest.mark.parametrize('env_value', ['FOO', '=FOO']) +def test_env_vars_malformed(ddev, helpers, data_dir, mocker, env_value): + invoke = mocker.patch('ddev.e2e.agent.docker.DockerAgent.invoke') + + integration = 'postgres' + environment = 'py3.12' + env_data = EnvDataStorage(data_dir).get(integration, environment) + env_data.write_metadata({}) + + result = ddev('env', 'agent', integration, environment, '--env', env_value, 'status') + + assert result.exit_code == 2, result.output + assert result.output == helpers.dedent( + f""" + Usage: ddev env agent [OPTIONS] INTEGRATION ENVIRONMENT ARGS... + + Error: Invalid value for '--env': `{env_value}` is not in KEY=VALUE format + """ + ) + + invoke.assert_not_called() + + +def test_env_vars_not_supported(ddev, helpers, data_dir, mocker): + invoke = mocker.patch( + 'ddev.e2e.agent.docker.DockerAgent.invoke', + side_effect=NotImplementedError('Per-invocation env_vars are not supported for the Vagrant agent'), + ) + + integration = 'postgres' + environment = 'py3.12' + env_data = EnvDataStorage(data_dir).get(integration, environment) + env_data.write_metadata({}) + + result = ddev('env', 'agent', integration, environment, '--env', 'FOO=bar', 'status') + + assert result.exit_code == 1, result.output + assert result.output == helpers.dedent( + """ + Per-invocation env_vars are not supported for the Vagrant agent + """ + ) + + invoke.assert_called_once_with(['status'], env_vars={'FOO': 'bar'}) + + +def test_env_vars_not_supported_trigger_run(ddev, helpers, data_dir, mocker): + invoke = mocker.patch( + 'ddev.e2e.agent.docker.DockerAgent.invoke', + side_effect=NotImplementedError('Per-invocation env_vars are not supported for the Vagrant agent'), + ) + + integration = 'postgres' + environment = 'py3.12' + env_data = EnvDataStorage(data_dir).get(integration, environment) + env_data.write_metadata({}) + + result = ddev('env', 'agent', integration, environment, '--env', 'FOO=bar', 'check', integration, '-l', 'debug') + + assert result.exit_code == 1, result.output + assert result.output == helpers.dedent( + """ + Per-invocation env_vars are not supported for the Vagrant agent + """ + ) + + invoke.assert_called_once_with(['check', integration, '-l', 'debug'], env_vars={'FOO': 'bar'}) def test_trigger_run(ddev, data_dir, mocker): @@ -51,7 +154,7 @@ def test_trigger_run(ddev, data_dir, mocker): assert result.exit_code == 0, result.output assert not result.output - invoke.assert_called_once_with(['check', integration, '-l', 'debug']) + invoke.assert_called_once_with(['check', integration, '-l', 'debug'], env_vars=None) def test_trigger_run_inject_integration(ddev, data_dir, mocker): @@ -67,4 +170,4 @@ def test_trigger_run_inject_integration(ddev, data_dir, mocker): assert result.exit_code == 0, result.output assert not result.output - invoke.assert_called_once_with(['check', integration, '-l', 'debug']) + invoke.assert_called_once_with(['check', integration, '-l', 'debug'], env_vars=None) diff --git a/ddev/tests/e2e/agent/test_docker.py b/ddev/tests/e2e/agent/test_docker.py index 809f025c909ea..651db9c85bf1f 100644 --- a/ddev/tests/e2e/agent/test_docker.py +++ b/ddev/tests/e2e/agent/test_docker.py @@ -1217,6 +1217,35 @@ def test_basic(self, app, get_integration, docker_path, mocker): ), ] + def test_with_env_vars(self, app, get_integration, docker_path, mocker): + run = mocker.patch('subprocess.run', return_value=mocker.MagicMock(returncode=0)) + + integration = 'postgres' + environment = 'py3.12' + metadata = {} + + agent = DockerAgent(app, get_integration(integration), environment, metadata, Path('config.yaml')) + agent.invoke(['check', 'postgres'], env_vars={'FOO': 'bar', 'BAZ': 'qux'}) + + assert run.call_args_list == [ + mocker.call( + [ + docker_path, + 'exec', + '-e', + 'FOO=bar', + '-e', + 'BAZ=qux', + f'dd_{integration}_{environment}', + 'agent', + 'check', + 'postgres', + ], + shell=False, + check=True, + ), + ] + class TestEnterShell: def test_linux_container(self, app, get_integration, docker_path, mocker): diff --git a/ddev/tests/helpers/clock.py b/ddev/tests/helpers/clock.py new file mode 100644 index 0000000000000..159a0aca301dc --- /dev/null +++ b/ddev/tests/helpers/clock.py @@ -0,0 +1,25 @@ +import asyncio + +import pytest + + +class FakeClock: + """Injectable, manually-advanceable clock for deterministic time in async tests.""" + + def __init__(self, start: float = 1000.0) -> None: + self.current = start + + def __call__(self) -> float: + return self.current + + def advance(self, seconds: float) -> None: + self.current += seconds + + +def advance_clock_on_sleep(clock: FakeClock, monkeypatch: pytest.MonkeyPatch) -> None: + """Make asyncio.sleep advance the fake clock instead of blocking, so timed waits are instant.""" + + async def fake_sleep(delay: float) -> None: + clock.advance(delay) + + monkeypatch.setattr(asyncio, "sleep", fake_sleep) diff --git a/ddev/tests/utils/github_async/AGENTS.md b/ddev/tests/utils/github_async/AGENTS.md new file mode 100644 index 0000000000000..20c8807c9f544 --- /dev/null +++ b/ddev/tests/utils/github_async/AGENTS.md @@ -0,0 +1,57 @@ +# Async GitHub Client Test Guidelines + +These rules govern the tests in this package. They supplement the repository root +`AGENTS.md`; where they overlap, the root file still applies. They exist because this suite +had grown with tests that could not fail for a reason living in the client code. + +## The one question before adding a test + +Can this test fail for a reason that lives in the client code? If the only way it fails is +Pydantic, `httpx`, or `ddev.utils.rate_limiting` breaking, it belongs in that layer's suite, +or nowhere. The client suite covers exactly: request construction (method, path, body, +headers), response wiring (parsing into the right model, forwarding headers), error +propagation, retry behavior, and the limiter/governor integration points. + +## Layer ownership + +- Model parsing contracts belong in `test_models.py`, exercised with `Model.model_validate(payload)` + and no transport or client. If a test only checks that a payload parses, it is a model test. +- Limiter and governor semantics belong in the rate-limiting suite (`tests/utils/test_rate_limiting.py`). + Here we test only that the client wires them in and reacts (`test_rate_limiting.py`). +- Enum exhaustiveness is never tested here. The client-owned boundary is "one valid member parses, + one invalid string raises." A request-constant, response-varying matrix over a `StrEnum` field + tests Pydantic, not the client. +- Helper modules obey the same layer boundaries as tests. Pure payload factories + live in `payloads.py` and import nothing beyond the stdlib; client-layer + helpers (transports, client factories, the endpoint registry) live in + `helpers.py` and may import payloads, never the reverse. `test_models.py` + imports only from `payloads.py` — if a model test needs something from + `helpers.py`, either the helper is misplaced or the test is not a model test. + +## Cross-cutting behavior goes through the registry + +Every public endpoint method must be registered in `ENDPOINT_CALLS` (in `helpers.py`) with minimal +valid arguments and a valid response factory. The registry drives error-propagation and +header-forwarding coverage for all endpoints at once. Never write a per-endpoint +`*_http_error_raises` or `*_headers_forwarded` test; register the method instead. + +## Per-endpoint tests + +Write exactly one success test per endpoint, asserting the request sent (method, path, body) and +the parsed model, plus tests for behavior unique to that endpoint (for example the workflow-dispatch +overloads, `update_check_run` conclusion validation, or the review-comment position/line-side +exclusivity). If a second endpoint test asserts the same things with different literals, it is a +duplicate. + +## Determinism + +- No `time.time()` in assertions. Governor-driven waits use `FakeClock` and `advance_clock_on_sleep` + from `tests/helpers/clock.py`. +- No explicit `@pytest.mark.asyncio`; asyncio auto mode is the convention. +- Signed-URL download flows route through `patch_signed_download`. + +## Assertions must be able to fail + +A test whose handler ignores the property named in the test is worse than no test: it reads as +coverage that isn't there. When adding a test, break the code mentally and confirm the assertion +would catch it. diff --git a/ddev/tests/utils/github_async/__init__.py b/ddev/tests/utils/github_async/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/ddev/tests/utils/github_async/helpers.py b/ddev/tests/utils/github_async/helpers.py new file mode 100644 index 0000000000000..93d89e0ba9bd1 --- /dev/null +++ b/ddev/tests/utils/github_async/helpers.py @@ -0,0 +1,170 @@ +"""Client-layer test helpers: transports, client factories, and the endpoint registry. + +May import from payloads.py for response bodies, never the reverse. Pure payload factories that +model tests use live in payloads.py so they carry no client, httpx, or limiter dependency. +""" + +from __future__ import annotations + +import dataclasses +import io +import zipfile +from collections.abc import AsyncIterator, Awaitable, Callable +from typing import Any + +import httpx +from aiolimiter import AsyncLimiter + +from ddev.utils.github_async import AsyncGitHubClient, GitHubResponse +from ddev.utils.rate_limiting import RATE_LIMIT_TIME_PERIOD, BudgetGovernor, InstrumentedAsyncLimiter +from tests.helpers.clock import FakeClock +from tests.utils.github_async.payloads import ( + artifact, + check_run_payload, + full_pull_request_payload, + issue_comment_payload, + pr_review_comment_payload, + pull_request_payload, + workflow_job, + workflow_run_payload, +) + +TOKEN = "ghp_test_token" + + +def json_response(data: Any, status_code: int = 200, headers: dict[str, str] | None = None) -> httpx.Response: + all_headers = {"content-type": "application/json"} + if headers: + all_headers.update(headers) + return httpx.Response(status_code, json=data, headers=all_headers) + + +def make_client(handler: httpx.MockTransport | None = None) -> AsyncGitHubClient: + transport = handler or httpx.MockTransport(lambda r: httpx.Response(200)) + return AsyncGitHubClient(token=TOKEN, transport=transport) + + +def make_zip(members: dict[str, bytes]) -> bytes: + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + for name, content in members.items(): + zf.writestr(name, content) + return buf.getvalue() + + +def patch_signed_download(monkeypatch, handler: Any) -> None: + """Route the anonymous signed-URL download through a mock transport.""" + real_async_client = httpx.AsyncClient + + def fake_async_client(*args: Any, **kwargs: Any) -> httpx.AsyncClient: + if kwargs.get("transport") is None: + kwargs["transport"] = httpx.MockTransport(handler) + return real_async_client(*args, **kwargs) + + monkeypatch.setattr("ddev.utils.github_async.client.httpx.AsyncClient", fake_async_client) + + +def recording_transport(items: list[httpx.Response | Exception]) -> tuple[httpx.MockTransport, list[httpx.Request]]: + """MockTransport that returns/raises *items* in order (last item repeats) and records each request.""" + calls: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + item = items[min(len(calls), len(items) - 1)] + calls.append(request) + if isinstance(item, Exception): + raise item + return item + + return httpx.MockTransport(handler), calls + + +def governed_client( + clock: FakeClock, + transport: httpx.MockTransport, + on_event: Any = None, + max_rate_limit_retries: int = 2, +) -> AsyncGitHubClient: + """Client whose governor runs on *clock*, so retry waits are deterministic under a fake sleep.""" + governor = BudgetGovernor(now=clock, on_event=on_event) + limiter = InstrumentedAsyncLimiter( + AsyncLimiter(max_rate=5000, time_period=RATE_LIMIT_TIME_PERIOD), + on_event=on_event, + budget_governor=governor, + name="github", + ) + return AsyncGitHubClient( + token=TOKEN, rate_limiter=limiter, transport=transport, max_rate_limit_retries=max_rate_limit_retries + ) + + +async def first_page(pages: AsyncIterator[GitHubResponse[Any]]) -> GitHubResponse[Any]: + """Return the first page of a paginated endpoint (enough to exercise errors and headers).""" + async for page in pages: + return page + raise AssertionError("expected at least one page") + + +@dataclasses.dataclass +class EndpointCase: + id: str + call: Callable[[AsyncGitHubClient], Awaitable[GitHubResponse[Any]]] + ok_response: Callable[[], httpx.Response] + + +ENDPOINT_CALLS = [ + EndpointCase( + "create_workflow_dispatch", + lambda c: c.create_workflow_dispatch("o", "r", "wf.yml", "main", return_run_details=True), + lambda: json_response( + {"workflow_run_id": 1, "run_url": "https://api.github.com/x", "html_url": "https://github.com/x"} + ), + ), + EndpointCase( + "get_workflow_run", lambda c: c.get_workflow_run("o", "r", 42), lambda: json_response(workflow_run_payload()) + ), + EndpointCase( + "list_workflow_run_artifacts", + lambda c: first_page(c.list_workflow_run_artifacts("o", "r", 1)), + lambda: json_response({"total_count": 1, "artifacts": [artifact(1)]}), + ), + EndpointCase( + "list_workflow_jobs", + lambda c: first_page(c.list_workflow_jobs("o", "r", 42)), + lambda: json_response({"total_count": 1, "jobs": [workflow_job(1)]}), + ), + EndpointCase( + "create_issue_comment", + lambda c: c.create_issue_comment("o", "r", 1, "body"), + lambda: json_response(issue_comment_payload()), + ), + EndpointCase( + "get_pull_request", + lambda c: c.get_pull_request("o", "r", 5), + lambda: json_response(full_pull_request_payload(number=5)), + ), + EndpointCase( + "create_pull_request", + lambda c: c.create_pull_request("o", "r", "t", "h", "b"), + lambda: json_response(pull_request_payload(number=1), status_code=201), + ), + EndpointCase( + "add_labels_to_issue", + lambda c: c.add_labels_to_issue("o", "r", 1, ["bug"]), + lambda: json_response([{"id": 1, "name": "bug"}]), + ), + EndpointCase( + "create_pr_review_comment", + lambda c: c.create_pr_review_comment("o", "r", 1, "body", "sha", "path", position=1), + lambda: json_response(pr_review_comment_payload()), + ), + EndpointCase( + "create_check_run", + lambda c: c.create_check_run("o", "r", "ck", "abc", "in_progress"), + lambda: json_response(check_run_payload()), + ), + EndpointCase( + "update_check_run", + lambda c: c.update_check_run("o", "r", 77, status="in_progress"), + lambda: json_response(check_run_payload(id=77)), + ), +] diff --git a/ddev/tests/utils/github_async/payloads.py b/ddev/tests/utils/github_async/payloads.py new file mode 100644 index 0000000000000..06c7428c64785 --- /dev/null +++ b/ddev/tests/utils/github_async/payloads.py @@ -0,0 +1,198 @@ +"""Pure GitHub API response payload factories. Stdlib-only: no httpx, models, or client imports.""" + +from __future__ import annotations + +from typing import Any + + +def artifact(idx: int, expired: bool = False, **extra: Any) -> dict[str, Any]: + return { + "id": idx, + "name": f"artifact-{idx}", + "size_in_bytes": 100 * idx, + "url": f"https://api.github.com/artifact/{idx}", + "archive_download_url": f"https://api.github.com/artifact/{idx}/zip", + "expired": expired, + **extra, + } + + +def workflow_run_payload( + id: int = 42, + name: str = "CI", + status: str = "completed", + conclusion: str | None = "success", + html_url: str = "https://github.com/owner/repo/actions/runs/42", + created_at: str = "2024-01-01T00:00:00Z", + updated_at: str = "2024-01-01T01:00:00Z", + **extra: Any, +) -> dict[str, Any]: + return { + "id": id, + "name": name, + "status": status, + "conclusion": conclusion, + "html_url": html_url, + "created_at": created_at, + "updated_at": updated_at, + **extra, + } + + +def workflow_job( + idx: int = 1, + run_id: int = 42, + name: str | None = None, + status: str = "completed", + conclusion: str | None = "success", + html_url: str | None = None, + steps: list[dict[str, Any]] | None = None, + **extra: Any, +) -> dict[str, Any]: + return { + "id": idx, + "run_id": run_id, + "name": name if name is not None else f"job-{idx}", + "status": status, + "conclusion": conclusion, + "html_url": html_url if html_url is not None else f"https://github.com/owner/repo/actions/runs/42/job/{idx}", + "steps": steps + if steps is not None + else [{"name": "Run tests", "status": "completed", "conclusion": "success", "number": 1}], + **extra, + } + + +def issue_comment_payload( + id: int = 1, + body: str = "Hello world", + user: dict[str, Any] | None = None, + created_at: str = "2024-01-01T00:00:00Z", + updated_at: str = "2024-01-01T00:00:00Z", + html_url: str = "https://github.com/owner/repo/issues/1#issuecomment-1", + **extra: Any, +) -> dict[str, Any]: + return { + "id": id, + "body": body, + "user": user if user is not None else {"login": "octocat"}, + "created_at": created_at, + "updated_at": updated_at, + "html_url": html_url, + **extra, + } + + +def pr_review_comment_payload( + id: int = 10, + body: str = "Nice change", + path: str = "src/foo.py", + commit_id: str = "abc123", + html_url: str = "https://github.com/owner/repo/pull/1#discussion_r10", + created_at: str = "2024-01-01T00:00:00Z", + updated_at: str = "2024-01-01T00:00:00Z", + user: dict[str, Any] | None = None, + **extra: Any, +) -> dict[str, Any]: + return { + "id": id, + "body": body, + "path": path, + "commit_id": commit_id, + "html_url": html_url, + "created_at": created_at, + "updated_at": updated_at, + "user": user if user is not None else {"login": "reviewer"}, + **extra, + } + + +def pull_request_payload(number: int = 1, html_url: str | None = None, **extra: Any) -> dict[str, Any]: + return { + "number": number, + "html_url": html_url if html_url is not None else f"https://github.com/owner/repo/pull/{number}", + **extra, + } + + +def full_pull_request_payload( + number: int = 42, + state: str = "open", + draft: bool = True, + merged: bool = False, + locked: bool = False, + title: str = "Fix bug", + body: str = "Backport", + node_id: str = "PR_kwDOABCD123", + merge_commit_sha: str | None = None, + created_at: str = "2026-05-01T00:00:00Z", + updated_at: str = "2026-05-02T00:00:00Z", + closed_at: str | None = None, + merged_at: str | None = None, + user: dict[str, Any] | None = None, + assignees: list[dict[str, Any]] | None = None, + requested_reviewers: list[dict[str, Any]] | None = None, + labels: list[dict[str, Any]] | None = None, + head: dict[str, Any] | None = None, + base: dict[str, Any] | None = None, + **extra: Any, +) -> dict[str, Any]: + """A richer PR payload exercising sub-models (user, labels, head/base).""" + return { + "id": 9000 + number, + "number": number, + "node_id": node_id, + "url": f"https://api.github.com/repos/owner/repo/pulls/{number}", + "html_url": f"https://github.com/owner/repo/pull/{number}", + "diff_url": f"https://github.com/owner/repo/pull/{number}.diff", + "patch_url": f"https://github.com/owner/repo/pull/{number}.patch", + "state": state, + "draft": draft, + "merged": merged, + "locked": locked, + "merge_commit_sha": merge_commit_sha, + "title": title, + "body": body, + "user": user + if user is not None + else {"id": 1, "login": "octocat", "html_url": "https://github.com/octocat", "type": "User"}, + "assignees": assignees if assignees is not None else [], + "requested_reviewers": requested_reviewers + if requested_reviewers is not None + else [{"id": 2, "login": "reviewer", "type": "User"}], + "labels": labels + if labels is not None + else [ + {"id": 100, "name": "qa/skip-qa", "color": "5319e7"}, + {"id": 101, "name": "backport/7.62.x", "color": "5319e7"}, + ], + "created_at": created_at, + "updated_at": updated_at, + "closed_at": closed_at, + "merged_at": merged_at, + "head": head + if head is not None + else {"ref": "alice/fix", "sha": "1234567890abcdef00", "label": "alice:alice/fix"}, + "base": base if base is not None else {"ref": "master", "sha": "cafebabe00", "label": "owner:master"}, + **extra, + } + + +def check_run_payload( + id: int = 1, + name: str = "ck", + status: str = "in_progress", + head_sha: str = "abc", + conclusion: str | None = None, + html_url: str = "https://github.com/o/r/check-runs/1", + **extra: Any, +) -> dict[str, Any]: + return { + "id": id, + "name": name, + "status": status, + "head_sha": head_sha, + "conclusion": conclusion, + "html_url": html_url, + **extra, + } diff --git a/ddev/tests/utils/github_async/test_client_core.py b/ddev/tests/utils/github_async/test_client_core.py new file mode 100644 index 0000000000000..cba077dfc9a23 --- /dev/null +++ b/ddev/tests/utils/github_async/test_client_core.py @@ -0,0 +1,124 @@ +"""Client construction, auth headers, context manager, timeout, and pagination mechanics.""" + +from __future__ import annotations + +import dataclasses +from typing import Any + +import httpx +import pytest + +from ddev.utils.github_async import GITHUB_API_VERSION, AsyncGitHubClient, PaginationData, async_github_client +from tests.utils.github_async.helpers import TOKEN, json_response, make_client +from tests.utils.github_async.payloads import artifact, workflow_run_payload + +BASE = "https://api.github.com" + + +@pytest.mark.parametrize( + ("header", "expected"), + [ + (None, PaginationData()), + ("", PaginationData()), + (f'<{BASE}/page2>; rel="next"', PaginationData(next=f"{BASE}/page2")), + (f'<{BASE}/page10>; rel="last"', PaginationData(last=f"{BASE}/page10")), + ( + f'<{BASE}/page2>; rel="next", <{BASE}/page5>; rel="last"', + PaginationData(next=f"{BASE}/page2", last=f"{BASE}/page5"), + ), + ( + f'<{BASE}/page1>; rel="first", <{BASE}/page1>; rel="prev",' + f' <{BASE}/page3>; rel="next", <{BASE}/page5>; rel="last"', + PaginationData(first=f"{BASE}/page1", prev=f"{BASE}/page1", next=f"{BASE}/page3", last=f"{BASE}/page5"), + ), + ( + f'<{BASE}/page1>; rel="prev", <{BASE}/page5>; rel="last"', + PaginationData(prev=f"{BASE}/page1", last=f"{BASE}/page5"), + ), + (f'<{BASE}/page1>; rel="first"', PaginationData(first=f"{BASE}/page1")), + ], + ids=["none", "blank", "next_only", "last_only", "next_and_last", "all_links", "prev_and_last", "first_only"], +) +def test_pagination_data_from_header(header: str | None, expected: PaginationData) -> None: + p = PaginationData.from_header(header) + for f in dataclasses.fields(expected): + assert getattr(p, f.name) == getattr(expected, f.name) + + +def test_client_empty_token_raises() -> None: + with pytest.raises(ValueError, match="token"): + AsyncGitHubClient(token="") + + +async def test_client_request_headers() -> None: + captured: httpx.Headers | None = None + + def handler(request: httpx.Request) -> httpx.Response: + nonlocal captured + captured = request.headers + return json_response(workflow_run_payload()) + + client = make_client(httpx.MockTransport(handler)) + await client.get_workflow_run("o", "r", 42) + + assert captured is not None + assert captured["authorization"] == f"Bearer {TOKEN}" + assert captured["x-github-api-version"] == GITHUB_API_VERSION + + +async def test_context_manager_yields_client() -> None: + async with async_github_client(token=TOKEN) as client: + assert not client._client.is_closed + + +async def test_context_manager_closes_on_exit() -> None: + async with async_github_client(token=TOKEN) as client: + inner = client._client + # After exit the underlying client is closed; a new request would fail + assert inner.is_closed + + +@pytest.mark.parametrize( + ("call_kwargs", "expected"), + [ + pytest.param({"timeout": 2.0}, 2.0, id="per-request-override"), + pytest.param({}, 5.0, id="constructor-default"), + ], +) +async def test_request_timeout_forwarded_to_transport(call_kwargs: dict[str, float], expected: float) -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured["timeout"] = request.extensions["timeout"] + return json_response(workflow_run_payload()) + + client = AsyncGitHubClient(token=TOKEN, default_timeout=5.0, transport=httpx.MockTransport(handler)) + await client.get_workflow_run("o", "r", 42, **call_kwargs) + + assert captured["timeout"] == dict.fromkeys(("connect", "read", "write", "pool"), expected) + + +async def test_list_workflow_run_artifacts_two_pages() -> None: + page1_artifacts = [artifact(1)] + page2_artifacts = [artifact(2)] + call_count = 0 + + def handler(request: httpx.Request) -> httpx.Response: + nonlocal call_count + call_count += 1 + if call_count == 1: + link = f'<{request.url.scheme}://{request.url.host}/page2>; rel="next"' + return json_response( + {"total_count": 2, "artifacts": page1_artifacts}, + headers={"link": link}, + ) + return json_response({"total_count": 2, "artifacts": page2_artifacts}) + + client = make_client(httpx.MockTransport(handler)) + pages = [] + async for page in client.list_workflow_run_artifacts("owner", "repo", 1): + pages.append(page) + + assert len(pages) == 2 + assert pages[0].data.artifacts[0].id == 1 + assert pages[1].data.artifacts[0].id == 2 diff --git a/ddev/tests/utils/github_async/test_download_artifact.py b/ddev/tests/utils/github_async/test_download_artifact.py new file mode 100644 index 0000000000000..792e1591b210c --- /dev/null +++ b/ddev/tests/utils/github_async/test_download_artifact.py @@ -0,0 +1,90 @@ +"""download_artifact: redirect resolution, token non-leak, zip-slip, signed-URL failures.""" + +from __future__ import annotations + +import httpx +import pytest + +from ddev.utils.github_async import AsyncGitHubClient +from tests.utils.github_async.helpers import TOKEN, make_client, make_zip, patch_signed_download + + +async def test_download_artifact_token_not_leaked_to_redirect_target(monkeypatch, tmp_path) -> None: + captured_signed_headers: dict[str, str] = {} + + def github_handler(request: httpx.Request) -> httpx.Response: + assert request.headers["authorization"].startswith("Bearer ") + return httpx.Response(302, headers={"location": "https://signed.example/zip"}) + + def signed_handler(request: httpx.Request) -> httpx.Response: + captured_signed_headers.update({k.lower(): v for k, v in request.headers.items()}) + return httpx.Response(200, content=make_zip({"hello.txt": b"hi"})) + + patch_signed_download(monkeypatch, signed_handler) + + client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") + + assert "authorization" not in captured_signed_headers + assert (tmp_path / "out" / "hello.txt").read_bytes() == b"hi" + + +async def test_download_artifact_non_302_raises(tmp_path) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=b"not a redirect") + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(httpx.HTTPError, match="Expected 302"): + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") + + +@pytest.mark.parametrize("status_code", [403, 503], ids=["forbidden", "server-error"]) +async def test_download_artifact_signed_url_error_propagates( + monkeypatch: pytest.MonkeyPatch, tmp_path, status_code: int +) -> None: + """A failed signed-URL download propagates as httpx.HTTPStatusError (no retries).""" + + def github_handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(302, headers={"location": "https://signed.example/zip"}) + + def signed_handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status_code, content=b"error") + + patch_signed_download(monkeypatch, signed_handler) + client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) + with pytest.raises(httpx.HTTPStatusError): + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") + + +async def test_download_artifact_missing_location_header_raises(tmp_path) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(302) + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(httpx.HTTPError, match="Missing Location"): + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") + + +@pytest.mark.parametrize( + "malicious_member", + [ + pytest.param("../escape.txt", id="parent-traversal"), + pytest.param("/etc/passwd", id="absolute-path"), + ], +) +async def test_download_artifact_zip_slip_rejected(monkeypatch, tmp_path, malicious_member: str) -> None: + def github_handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(302, headers={"location": "https://signed.example/zip"}) + + def signed_handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=make_zip({malicious_member: b"pwn"})) + + patch_signed_download(monkeypatch, signed_handler) + + client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) + dest = tmp_path / "out" + with pytest.raises(ValueError, match="(?i)zip-slip"): + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", dest) + + # Nothing was extracted before the guard fired. + assert list(dest.rglob("*")) == [] diff --git a/ddev/tests/utils/github_async/test_endpoints.py b/ddev/tests/utils/github_async/test_endpoints.py new file mode 100644 index 0000000000000..934408f84fc2d --- /dev/null +++ b/ddev/tests/utils/github_async/test_endpoints.py @@ -0,0 +1,425 @@ +"""Per-endpoint success tests plus the registry-driven cross-cutting error/header tests.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING, Any + +import httpx +import pytest +from pydantic import ValidationError + +from ddev.utils.github_async import GitHubResponse +from ddev.utils.github_async.models import ( + ArtifactsList, + CheckRun, + CheckRunConclusion, + CheckRunStatus, + GitHubUser, + IssueComment, + JobStep, + JobStepStatus, + Label, + PullRequest, + PullRequestReviewComment, + PullRequestState, + WorkflowDispatchResult, + WorkflowJob, + WorkflowJobConclusion, + WorkflowJobsList, + WorkflowJobStatus, + WorkflowRun, +) +from tests.utils.github_async.helpers import ENDPOINT_CALLS, json_response, make_client +from tests.utils.github_async.payloads import ( + artifact, + check_run_payload, + full_pull_request_payload, + issue_comment_payload, + pr_review_comment_payload, + pull_request_payload, + workflow_job, + workflow_run_payload, +) + +if TYPE_CHECKING: + from tests.utils.github_async.helpers import EndpointCase + + +async def test_create_workflow_dispatch_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert "/actions/workflows/my-workflow.yml/dispatches" in request.url.path + body = json.loads(request.content) + assert body["ref"] == "main" + assert body["return_run_details"] is True + assert "inputs" not in body + return json_response( + { + "workflow_run_id": 999, + "run_url": "https://api.github.com/repos/owner/repo/actions/runs/999", + "html_url": "https://github.com/owner/repo/actions/runs/999", + }, + headers={"x-ratelimit-remaining": "59"}, + ) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_workflow_dispatch("owner", "repo", "my-workflow.yml", "main", return_run_details=True) + assert isinstance(result, GitHubResponse) + assert isinstance(result.data, WorkflowDispatchResult) + assert result.data.workflow_run_id == 999 + assert result.headers.get("x-ratelimit-remaining") == "59" + + +async def test_create_workflow_dispatch_with_inputs() -> None: + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + assert body["inputs"] == {"env": "prod"} + assert body["return_run_details"] is True + return json_response( + { + "workflow_run_id": 1, + "run_url": "https://api.github.com/repos/o/r/actions/runs/1", + "html_url": "https://github.com/o/r/actions/runs/1", + } + ) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_workflow_dispatch( + "o", "r", 123, "main", inputs={"env": "prod"}, return_run_details=True + ) + assert result.data.workflow_run_id == 1 + + +async def test_create_workflow_dispatch_omits_return_run_details_when_false() -> None: + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + assert "return_run_details" not in body + return httpx.Response(204) + + client = make_client(httpx.MockTransport(handler)) + await client.create_workflow_dispatch("o", "r", "wf.yml", "main") + + +# --------------------------------------------------------------------------- +# get_workflow_run +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("status", "is_completed"), + [("completed", True), ("in_progress", False), ("queued", False)], + ids=["completed", "in_progress", "queued"], +) +async def test_get_workflow_run_success(status: str, is_completed: bool) -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "GET" + assert "/actions/runs/42" in request.url.path + return json_response(workflow_run_payload(status=status)) + + client = make_client(httpx.MockTransport(handler)) + result = await client.get_workflow_run("owner", "repo", 42) + assert isinstance(result.data, WorkflowRun) + assert result.data.id == 42 + assert result.data.status == status + assert result.data.is_completed is is_completed + + +async def test_list_workflow_run_artifacts_single_page() -> None: + artifacts = [artifact(1), artifact(2)] + + def handler(_: httpx.Request) -> httpx.Response: + return json_response({"total_count": 2, "artifacts": artifacts}) + + client = make_client(httpx.MockTransport(handler)) + pages = [] + async for page in client.list_workflow_run_artifacts("owner", "repo", 1): + pages.append(page) + + assert len(pages) == 1 + assert isinstance(pages[0].data, ArtifactsList) + assert pages[0].data.total_count == 2 + assert len(pages[0].data.artifacts) == 2 + + +async def test_list_workflow_run_artifacts_per_page_forwarded() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.params["per_page"] == "100" + return json_response({"total_count": 0, "artifacts": []}) + + client = make_client(httpx.MockTransport(handler)) + async for _ in client.list_workflow_run_artifacts("owner", "repo", 1, per_page=100): + pass + + +async def test_list_workflow_jobs_single_page() -> None: + jobs = [workflow_job(1), workflow_job(2, status="in_progress", conclusion=None)] + + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "GET" + assert "/actions/runs/42/jobs" in request.url.path + return json_response({"total_count": 2, "jobs": jobs}) + + client = make_client(httpx.MockTransport(handler)) + pages = [] + async for page in client.list_workflow_jobs("owner", "repo", 42): + pages.append(page) + + assert len(pages) == 1 + assert isinstance(pages[0].data, WorkflowJobsList) + assert pages[0].data.total_count == 2 + + first, second = pages[0].data.jobs + assert isinstance(first, WorkflowJob) + assert first.status is WorkflowJobStatus.COMPLETED + assert first.conclusion is WorkflowJobConclusion.SUCCESS + assert second.status is WorkflowJobStatus.IN_PROGRESS + assert second.conclusion is None + assert second.html_url == "https://github.com/owner/repo/actions/runs/42/job/2" + + # A step's status is its own StrEnum; its conclusion has no declared enum (stays str). + step = first.steps[0] + assert isinstance(step, JobStep) + assert step.status is JobStepStatus.COMPLETED + + +async def test_list_workflow_jobs_unexpected_status_raises() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return json_response({"total_count": 1, "jobs": [workflow_job(1, status="bogus")]}) + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(ValidationError): + async for _ in client.list_workflow_jobs("owner", "repo", 42): + pass + + +async def test_list_workflow_jobs_per_page_forwarded() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.params["per_page"] == "100" + return json_response({"total_count": 0, "jobs": []}) + + client = make_client(httpx.MockTransport(handler)) + async for _ in client.list_workflow_jobs("owner", "repo", 42, per_page=100): + pass + + +async def test_create_issue_comment_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert "/issues/7/comments" in request.url.path + body = json.loads(request.content) + assert body["body"] == "LGTM" + return json_response(issue_comment_payload(body="LGTM"), status_code=201) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_issue_comment("owner", "repo", 7, "LGTM") + assert isinstance(result.data, IssueComment) + assert result.data.body == "LGTM" + assert isinstance(result.data.user, GitHubUser) + assert result.data.user.login == "octocat" + + +async def test_create_pr_review_comment_success_with_position() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert "/pulls/3/comments" in request.url.path + body = json.loads(request.content) + assert body["commit_id"] == "abc123" + assert body["path"] == "src/foo.py" + assert body["position"] == 5 + assert "line" not in body + return json_response(pr_review_comment_payload(), status_code=201) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_pr_review_comment( + "owner", "repo", 3, "Nice change", "abc123", "src/foo.py", position=5 + ) + assert isinstance(result.data, PullRequestReviewComment) + assert result.data.commit_id == "abc123" + assert isinstance(result.data.user, GitHubUser) + assert result.data.user.login == "reviewer" + + +async def test_create_pr_review_comment_success_with_line_and_side() -> None: + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + assert body["line"] == 10 + assert body["side"] == "RIGHT" + assert "position" not in body + return json_response(pr_review_comment_payload(), status_code=201) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_pr_review_comment("o", "r", 1, "comment", "abc123", "file.py", line=10, side="RIGHT") + assert isinstance(result.data, PullRequestReviewComment) + + +async def test_create_pull_request_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert "/repos/owner/repo/pulls" in request.url.path + body = json.loads(request.content) + assert body == { + "title": "Fix bug", + "head": "alice/fix", + "base": "master", + "body": "Fix description", + "draft": False, + } + return json_response(pull_request_payload(number=42), status_code=201) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_pull_request("owner", "repo", "Fix bug", "alice/fix", "master", "Fix description") + assert isinstance(result.data, PullRequest) + assert result.data.number == 42 + assert result.data.html_url == "https://github.com/owner/repo/pull/42" + + +async def test_create_pull_request_draft_true_forwarded() -> None: + def handler(request: httpx.Request) -> httpx.Response: + body = json.loads(request.content) + assert body["draft"] is True + return json_response(pull_request_payload(number=7), status_code=201) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_pull_request("o", "r", "T", "h", "b", draft=True) + assert result.data.number == 7 + + +async def test_get_pull_request_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "GET" + assert "/repos/owner/repo/pulls/5" in request.url.path + return json_response(full_pull_request_payload(number=5, state="open")) + + client = make_client(httpx.MockTransport(handler)) + result = await client.get_pull_request("owner", "repo", 5) + assert isinstance(result.data, PullRequest) + assert result.data.number == 5 + assert result.data.state is PullRequestState.OPEN + + +async def test_get_pull_request_unexpected_state_raises() -> None: + def handler(_: httpx.Request) -> httpx.Response: + return json_response(full_pull_request_payload(number=5, state="merged")) + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(ValidationError): + await client.get_pull_request("o", "r", 5) + + +async def test_add_labels_to_issue_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert "/repos/owner/repo/issues/3/labels" in request.url.path + body = json.loads(request.content) + assert body == {"labels": ["qa/skip-qa", "backport/7.62.x"]} + return json_response([{"id": 1, "name": "qa/skip-qa"}, {"id": 2, "name": "backport/7.62.x"}], status_code=200) + + client = make_client(httpx.MockTransport(handler)) + result = await client.add_labels_to_issue("owner", "repo", 3, ["qa/skip-qa", "backport/7.62.x"]) + assert [label.name for label in result.data] == ["qa/skip-qa", "backport/7.62.x"] + assert all(isinstance(label, Label) for label in result.data) + + +async def test_create_check_run_success() -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "POST" + assert "/repos/o/r/check-runs" in request.url.path + captured.update(json.loads(request.content)) + return json_response(check_run_payload(name=captured["name"], head_sha=captured["head_sha"])) + + client = make_client(httpx.MockTransport(handler)) + result = await client.create_check_run("o", "r", name="ck", head_sha="abc", status="in_progress") + assert isinstance(result.data, CheckRun) + assert result.data.id == 1 + assert result.data.status is CheckRunStatus.IN_PROGRESS + assert captured == {"name": "ck", "head_sha": "abc", "status": "in_progress"} + + +async def test_create_check_run_with_optional_fields() -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured.update(json.loads(request.content)) + return json_response(check_run_payload()) + + client = make_client(httpx.MockTransport(handler)) + await client.create_check_run( + "o", + "r", + name="ck", + head_sha="abc", + status="in_progress", + details_url="https://x", + output={"title": "t", "summary": "s"}, + ) + assert captured["details_url"] == "https://x" + assert captured["output"] == {"title": "t", "summary": "s"} + + +async def test_update_check_run_success() -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.method == "PATCH" + assert "/repos/o/r/check-runs/77" in request.url.path + return json_response(check_run_payload(id=77, status="completed", conclusion="success")) + + client = make_client(httpx.MockTransport(handler)) + result = await client.update_check_run("o", "r", 77, status="completed", conclusion="success") + assert result.data.status is CheckRunStatus.COMPLETED + assert result.data.conclusion is CheckRunConclusion.SUCCESS + + +async def test_update_check_run_omits_unset_fields() -> None: + captured: dict[str, Any] = {} + + def handler(request: httpx.Request) -> httpx.Response: + captured.update(json.loads(request.content)) + return json_response(check_run_payload(id=77)) + + client = make_client(httpx.MockTransport(handler)) + await client.update_check_run("o", "r", 77, conclusion="failure") + assert captured == {"conclusion": "failure"} + + +async def test_update_check_run_requires_conclusion_when_completed() -> None: + requested = False + + def handler(request: httpx.Request) -> httpx.Response: + nonlocal requested + requested = True + return json_response(check_run_payload()) + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(ValueError, match="conclusion is required"): + await client.update_check_run("o", "r", 77, status="completed") + assert requested is False + + +async def test_update_check_run_unexpected_status_raises() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return json_response(check_run_payload(id=77, status="bogus")) + + client = make_client(httpx.MockTransport(handler)) + with pytest.raises(ValidationError): + await client.update_check_run("o", "r", 77, status="in_progress") + + +@pytest.mark.parametrize("case", ENDPOINT_CALLS, ids=[case.id for case in ENDPOINT_CALLS]) +async def test_endpoint_http_error_raises(case: EndpointCase) -> None: + client = make_client(httpx.MockTransport(lambda r: httpx.Response(422))) + with pytest.raises(httpx.HTTPStatusError) as exc_info: + await case.call(client) + assert exc_info.value.response.status_code == 422 + + +@pytest.mark.parametrize("case", ENDPOINT_CALLS, ids=[case.id for case in ENDPOINT_CALLS]) +async def test_endpoint_forwards_response_headers(case: EndpointCase) -> None: + def handler(request: httpx.Request) -> httpx.Response: + response = case.ok_response() + response.headers["x-ratelimit-remaining"] = "42" + return response + + client = make_client(httpx.MockTransport(handler)) + result = await case.call(client) + assert result.headers["x-ratelimit-remaining"] == "42" diff --git a/ddev/tests/utils/github_async/test_models.py b/ddev/tests/utils/github_async/test_models.py new file mode 100644 index 0000000000000..fa61793a01274 --- /dev/null +++ b/ddev/tests/utils/github_async/test_models.py @@ -0,0 +1,85 @@ +"""Tests for the github_async response models: parsing contracts, no transport or client.""" + +from __future__ import annotations + +import pytest + +from ddev.utils.github_async.models import ( + GitHubUser, + Label, + PullRequest, + PullRequestRef, + PullRequestState, +) +from tests.utils.github_async.payloads import full_pull_request_payload + + +def test_pull_request_parses_full_response() -> None: + """PullRequest parses its sub-models (GitHubUser, Label, PullRequestRef) end-to-end.""" + pr = PullRequest.model_validate(full_pull_request_payload(number=42)) + + assert pr.id == 9042 + assert pr.number == 42 + assert pr.state is PullRequestState.OPEN + assert pr.draft is True + assert pr.title == "Fix bug" + + assert isinstance(pr.user, GitHubUser) + assert pr.user.login == "octocat" + + assert [label.name for label in pr.labels] == ["qa/skip-qa", "backport/7.62.x"] + assert all(isinstance(label, Label) for label in pr.labels) + + assert isinstance(pr.head, PullRequestRef) + assert pr.head.ref == "alice/fix" + assert pr.head.sha == "1234567890abcdef00" + assert isinstance(pr.base, PullRequestRef) + assert pr.base.ref == "master" + + assert [reviewer.login for reviewer in pr.requested_reviewers] == ["reviewer"] + assert pr.created_at == "2026-05-01T00:00:00Z" + + +def test_pull_request_ignores_extra_fields() -> None: + """Unknown top-level fields in the response must not break parsing.""" + payload = full_pull_request_payload(mergeable_state="clean", additions=42, unknown_future_field={"nested": True}) + pr = PullRequest.model_validate(payload) + assert pr.number == 42 + + +def test_models_subpackage_unknown_attribute_raises_attribute_error() -> None: + import ddev.utils.github_async.models as models + + with pytest.raises(AttributeError, match="no attribute"): + models.NotARealModel # noqa: B018 + + +def test_models_subpackage_loads_only_requested_submodule() -> None: + """Importing one model must not eagerly load every other model submodule. + + Runs in a clean subprocess so the import effect is observable (the parent test process + has already loaded everything for other tests). + """ + import subprocess + import sys + import textwrap + + script = textwrap.dedent( + """ + import sys + from ddev.utils.github_async.models import PullRequest # noqa: F401 + + assert 'ddev.utils.github_async.client' not in sys.modules, 'client module should not be loaded' + assert 'httpx' not in sys.modules, 'httpx should not be loaded when only models are imported' + + prefix = 'ddev.utils.github_async.models.' + loaded = sorted(name[len(prefix):] for name in sys.modules if name.startswith(prefix)) + print(','.join(loaded)) + """ + ) + result = subprocess.run([sys.executable, '-c', script], capture_output=True, text=True, check=True) + loaded = set(result.stdout.strip().split(',')) + + assert {'pull_request', 'user', 'label'} <= loaded + assert 'workflow' not in loaded + assert 'comment' not in loaded diff --git a/ddev/tests/utils/github_async/test_rate_limiting.py b/ddev/tests/utils/github_async/test_rate_limiting.py new file mode 100644 index 0000000000000..56b20cd19a69a --- /dev/null +++ b/ddev/tests/utils/github_async/test_rate_limiting.py @@ -0,0 +1,304 @@ +"""Rate-limit wiring: snapshot parsing, governor observation, default construction, and retries.""" + +from __future__ import annotations + +import dataclasses +import logging + +import httpx +import pytest +from aiolimiter import AsyncLimiter + +from ddev.utils.github_async import AsyncGitHubClient, async_github_client +from ddev.utils.github_async.client import github_rate_limit_snapshot +from ddev.utils.github_async.defaults import default_github_rate_limiter, log_rate_limit_events +from ddev.utils.github_async.models import WorkflowRun +from ddev.utils.rate_limiting import ( + BucketEvent, + BudgetGovernor, + BudgetSnapshot, + InstrumentedAsyncLimiter, + PacingEvent, + PacingReason, + RateLimitEvent, + SecondaryLimitEvent, +) +from tests.helpers.clock import FakeClock, advance_clock_on_sleep +from tests.utils.github_async.helpers import ( + TOKEN, + governed_client, + json_response, + make_zip, + patch_signed_download, + recording_transport, +) +from tests.utils.github_async.payloads import workflow_run_payload + +LOGGER_NAME = "ddev.utils.github_async.defaults" + + +async def test_client_request_with_rate_limiter_consumes_token() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return json_response(workflow_run_payload()) + + real_limiter = AsyncLimiter(max_rate=1, time_period=1000) + events: list[RateLimitEvent] = [] + rate_limiter = InstrumentedAsyncLimiter(real_limiter, on_event=events.append) + + async with async_github_client( + token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) + ) as client: + result = await client.get_workflow_run("o", "r", 42) + + assert result.data.id == 42 + bucket_events = [event for event in events if isinstance(event, BucketEvent)] + assert bucket_events == [BucketEvent(throttled=False, name="")] + assert not real_limiter.has_capacity() + + +@pytest.mark.parametrize( + ("headers", "expected"), + [ + ( + { + "x-ratelimit-limit": "5000", + "x-ratelimit-remaining": "4321", + "x-ratelimit-reset": "1700000000", + "retry-after": "30", + }, + BudgetSnapshot(limit=5000, remaining=4321, reset_at=1700000000.0, retry_after=30.0), + ), + ( + {"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4999"}, + BudgetSnapshot(limit=5000, remaining=4999), + ), + ({"x-ratelimit-limit": "5000", "retry-after": "not-a-number"}, BudgetSnapshot(limit=5000)), + ({"x-ratelimit-limit": "not-a-number"}, None), + ({"content-type": "application/json"}, None), + ], + ids=["all_present", "partial_primary", "non_integer_retry_after", "all_unparseable", "no_ratelimit_headers"], +) +def test_github_rate_limit_snapshot(headers: dict[str, str], expected: BudgetSnapshot | None) -> None: + assert github_rate_limit_snapshot(httpx.Headers(headers)) == expected + + +async def test_client_request_observes_rate_limit_headers_into_governor() -> None: + def handler(request: httpx.Request) -> httpx.Response: + return json_response( + workflow_run_payload(), + headers={"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4999", "x-ratelimit-reset": "1700000000"}, + ) + + governor = BudgetGovernor() + rate_limiter = InstrumentedAsyncLimiter(AsyncLimiter(max_rate=1, time_period=1000), budget_governor=governor) + + async with async_github_client( + token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) + ) as client: + await client.get_workflow_run("o", "r", 42) + + assert governor.budget.limit == 5000 + assert governor.budget.remaining == 4999 + assert governor.budget.reset_at == 1700000000.0 + + +async def test_default_rate_limiter_is_constructed_and_observes_403() -> None: + """rate_limiter=None builds a limiter with a governor that observes a 403's retry-after.""" + transport, calls = recording_transport([httpx.Response(403, headers={"retry-after": "30"})]) + client = AsyncGitHubClient(token=TOKEN, transport=transport, max_rate_limit_retries=0) + + assert client._rate_limiter is not None + governor = client._rate_limiter.budget_governor + assert governor is not None + + with pytest.raises(httpx.HTTPStatusError): + await client._request("GET", "/x") + + # The 403's retry-after was observed (before raise_for_status), arming the shared pause; + # exact pause arithmetic is covered by the clocked governor tests. + assert governor.pause_until > 0 + assert len(calls) == 1 # retries disabled: one call, no wait + + +async def test_retry_on_secondary_limit_returns_success(monkeypatch: pytest.MonkeyPatch) -> None: + """A 403 with retry-after then a 200 is retried once and the wait goes through the governor.""" + clock = FakeClock() + advance_clock_on_sleep(clock, monkeypatch) + events: list[RateLimitEvent] = [] + transport, calls = recording_transport([httpx.Response(403, headers={"retry-after": "5"}), httpx.Response(200)]) + client = governed_client(clock, transport, on_event=events.append) + + response = await client._request("GET", "/x") + + assert response.status_code == 200 + assert len(calls) == 2 + secondary_index = next(i for i, e in enumerate(events) if isinstance(e, SecondaryLimitEvent)) + pacing_index = next( + i for i, e in enumerate(events) if isinstance(e, PacingEvent) and e.reason is PacingReason.SECONDARY_LIMIT + ) + assert secondary_index < pacing_index + + +async def test_retry_on_primary_exhaustion_waits_until_reset(monkeypatch: pytest.MonkeyPatch) -> None: + """A 403 with x-ratelimit-remaining=0 is retried, and the retry waits until the window reset.""" + clock = FakeClock() + advance_clock_on_sleep(clock, monkeypatch) + events: list[RateLimitEvent] = [] + reset_at = clock.current + 30 + transport, calls = recording_transport( + [ + httpx.Response( + 403, + headers={"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "0", "x-ratelimit-reset": str(reset_at)}, + ), + httpx.Response(200), + ] + ) + client = governed_client(clock, transport, on_event=events.append) + + response = await client._request("GET", "/x") + + assert response.status_code == 200 + assert len(calls) == 2 + governor = client._rate_limiter.budget_governor + assert clock.current == pytest.approx(reset_at + governor.buffer_seconds) + assert any(isinstance(e, PacingEvent) and e.reason is PacingReason.EXHAUSTED for e in events) + + +async def test_no_retry_on_permission_denied_403() -> None: + """A 403 with no retry-after and nonzero remaining is a permission denial: raise on first attempt.""" + transport, calls = recording_transport([httpx.Response(403, headers={"x-ratelimit-remaining": "5"})]) + client = AsyncGitHubClient(token=TOKEN, transport=transport) + + with pytest.raises(httpx.HTTPStatusError): + await client._request("GET", "/x") + + assert len(calls) == 1 + + +async def test_no_retry_on_transport_error() -> None: + """A transport error is never retried (the action may have executed); it propagates immediately.""" + transport, calls = recording_transport([httpx.ConnectError("boom")]) + client = AsyncGitHubClient(token=TOKEN, transport=transport) + + with pytest.raises(httpx.ConnectError): + await client._request("GET", "/x") + + assert len(calls) == 1 + + +async def test_retries_exhausted_raises_after_max(monkeypatch: pytest.MonkeyPatch) -> None: + """Two consecutive rate-limit responses with max_rate_limit_retries=1 raise after exactly two calls.""" + clock = FakeClock() + advance_clock_on_sleep(clock, monkeypatch) + transport, calls = recording_transport( + [httpx.Response(403, headers={"retry-after": "5"}), httpx.Response(403, headers={"retry-after": "5"})] + ) + client = governed_client(clock, transport, max_rate_limit_retries=1) + + with pytest.raises(httpx.HTTPStatusError): + await client._request("GET", "/x") + + assert len(calls) == 2 + + +async def test_download_redirect_302_is_not_retried(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: + """The artifact 302 redirect is not a rate-limit response, so it resolves without any retry.""" + github_calls: list[httpx.Request] = [] + + def github_handler(request: httpx.Request) -> httpx.Response: + github_calls.append(request) + return httpx.Response(302, headers={"location": "https://signed.example/zip"}) + + def signed_handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=make_zip({"hello.txt": b"hi"})) + + patch_signed_download(monkeypatch, signed_handler) + client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) + + await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") + + assert len(github_calls) == 1 + + +async def test_pagination_retries_only_the_rate_limited_page(monkeypatch: pytest.MonkeyPatch) -> None: + """A rate-limited page is retried in place; the iterator then continues to the next page.""" + clock = FakeClock() + advance_clock_on_sleep(clock, monkeypatch) + transport, calls = recording_transport( + [ + httpx.Response(200, json={"page": 1}, headers={"link": '; rel="next"'}), + httpx.Response(403, headers={"retry-after": "5"}), + httpx.Response(200, json={"page": 2}), + ] + ) + client = governed_client(clock, transport) + + pages = [response async for response in client._paginated_request("GET", "/start")] + + assert [page.status_code for page in pages] == [200, 200] + assert len(calls) == 3 # page 1, page 2 (rate-limited), page 2 (retry) + + +async def test_endpoint_retries_rate_limit_then_parses(monkeypatch: pytest.MonkeyPatch) -> None: + """A public endpoint call retries a rate-limit response and returns the parsed model.""" + clock = FakeClock() + advance_clock_on_sleep(clock, monkeypatch) + transport, calls = recording_transport( + [httpx.Response(403, headers={"retry-after": "5"}), json_response(workflow_run_payload())] + ) + client = governed_client(clock, transport) + + result = await client.get_workflow_run("o", "r", 42) + + assert isinstance(result.data, WorkflowRun) + assert result.data.id == 42 + assert len(calls) == 2 + + +async def test_default_github_rate_limiter_wires_callback_into_both_slots() -> None: + """The factory must wire the callback into both the bucket and the governor slots.""" + seen: list[RateLimitEvent] = [] + limiter = default_github_rate_limiter(on_event=seen.append) + + await limiter.__aenter__() # bucket acquire -> BucketEvent (limiter slot) + limiter.observe(BudgetSnapshot(limit=100, remaining=10, reset_at=2000.0)) # observe -> BudgetEvent (governor slot) + + kinds = {type(event).__name__ for event in seen} + assert "BucketEvent" in kinds # would be missing if the limiter slot were unwired + assert "BudgetEvent" in kinds # would be missing if the governor slot were unwired + + +@pytest.mark.parametrize( + ("event", "expected_level"), + [ + pytest.param( + SecondaryLimitEvent(retry_after_seconds=5.0, pause_seconds=6.0), logging.WARNING, id="secondary_limit" + ), + pytest.param(PacingEvent(wait_seconds=0.0, reason=PacingReason.NONE), logging.DEBUG, id="healthy_pacing"), + pytest.param(PacingEvent(wait_seconds=3.0, reason=PacingReason.RATIONING), logging.INFO, id="rationing"), + pytest.param(PacingEvent(wait_seconds=9.0, reason=PacingReason.EXHAUSTED), logging.WARNING, id="exhausted"), + pytest.param(PacingEvent(wait_seconds=9.0, reason=PacingReason.ABANDONED), logging.ERROR, id="abandoned"), + ], +) +def test_log_rate_limit_events_level_mapping( + caplog: pytest.LogCaptureFixture, event: RateLimitEvent, expected_level: int +) -> None: + with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): + log_rate_limit_events()(event) + + assert caplog.records + assert caplog.records[-1].levelno == expected_level + + +def test_log_rate_limit_events_unknown_event_does_not_raise(caplog: pytest.LogCaptureFixture) -> None: + """A future event type must fall through to the DEBUG catch-all, never raise.""" + + @dataclasses.dataclass(frozen=True) + class FutureEvent: + type: str = "future" + + with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): + log_rate_limit_events()(FutureEvent()) # type: ignore[arg-type] + + assert caplog.records[-1].levelno == logging.DEBUG diff --git a/ddev/tests/utils/test_github_async.py b/ddev/tests/utils/test_github_async.py deleted file mode 100644 index 95f7f853493f5..0000000000000 --- a/ddev/tests/utils/test_github_async.py +++ /dev/null @@ -1,1518 +0,0 @@ -"""Tests for the async GitHub API client.""" - -from __future__ import annotations - -import asyncio -import dataclasses -import io -import json -import time -import zipfile -from typing import Any - -import httpx -import pytest -from aiolimiter import AsyncLimiter -from pydantic import ValidationError - -from ddev.utils.github_async import ( - GITHUB_API_VERSION, - AsyncGitHubClient, - GitHubResponse, - PaginationData, - async_github_client, -) -from ddev.utils.github_async.client import github_rate_limit_snapshot -from ddev.utils.github_async.models import ( - ArtifactsList, - CheckRun, - CheckRunConclusion, - CheckRunStatus, - GitHubUser, - IssueComment, - JobStep, - JobStepStatus, - Label, - PullRequest, - PullRequestRef, - PullRequestReviewComment, - PullRequestState, - WorkflowDispatchResult, - WorkflowJob, - WorkflowJobConclusion, - WorkflowJobsList, - WorkflowJobStatus, - WorkflowRun, -) -from ddev.utils.rate_limiting import ( - RATE_LIMIT_TIME_PERIOD, - BucketEvent, - BudgetGovernor, - BudgetSnapshot, - InstrumentedAsyncLimiter, - PacingEvent, - PacingReason, - RateLimitEvent, - SecondaryLimitEvent, -) - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - -TOKEN = "ghp_test_token" - - -def json_response(data: Any, status_code: int = 200, headers: dict[str, str] | None = None) -> httpx.Response: - all_headers = {"content-type": "application/json"} - if headers: - all_headers.update(headers) - return httpx.Response(status_code, json=data, headers=all_headers) - - -def make_client(handler: httpx.MockTransport | None = None) -> AsyncGitHubClient: - transport = handler or httpx.MockTransport(lambda r: httpx.Response(200)) - return AsyncGitHubClient(token=TOKEN, transport=transport) - - -def artifact(idx: int, expired: bool = False, **extra: Any) -> dict[str, Any]: - return { - "id": idx, - "name": f"artifact-{idx}", - "size_in_bytes": 100 * idx, - "url": f"https://api.github.com/artifact/{idx}", - "archive_download_url": f"https://api.github.com/artifact/{idx}/zip", - "expired": expired, - **extra, - } - - -def workflow_run_payload( - id: int = 42, - name: str = "CI", - status: str = "completed", - conclusion: str | None = "success", - html_url: str = "https://github.com/owner/repo/actions/runs/42", - created_at: str = "2024-01-01T00:00:00Z", - updated_at: str = "2024-01-01T01:00:00Z", - **extra: Any, -) -> dict[str, Any]: - return { - "id": id, - "name": name, - "status": status, - "conclusion": conclusion, - "html_url": html_url, - "created_at": created_at, - "updated_at": updated_at, - **extra, - } - - -def workflow_job( - idx: int = 1, - run_id: int = 42, - name: str | None = None, - status: str = "completed", - conclusion: str | None = "success", - html_url: str | None = None, - steps: list[dict[str, Any]] | None = None, - **extra: Any, -) -> dict[str, Any]: - return { - "id": idx, - "run_id": run_id, - "name": name if name is not None else f"job-{idx}", - "status": status, - "conclusion": conclusion, - "html_url": html_url if html_url is not None else f"https://github.com/owner/repo/actions/runs/42/job/{idx}", - "steps": steps - if steps is not None - else [{"name": "Run tests", "status": "completed", "conclusion": "success", "number": 1}], - **extra, - } - - -def issue_comment_payload( - id: int = 1, - body: str = "Hello world", - user: dict[str, Any] | None = None, - created_at: str = "2024-01-01T00:00:00Z", - updated_at: str = "2024-01-01T00:00:00Z", - html_url: str = "https://github.com/owner/repo/issues/1#issuecomment-1", - **extra: Any, -) -> dict[str, Any]: - return { - "id": id, - "body": body, - "user": user if user is not None else {"login": "octocat"}, - "created_at": created_at, - "updated_at": updated_at, - "html_url": html_url, - **extra, - } - - -def pr_review_comment_payload( - id: int = 10, - body: str = "Nice change", - path: str = "src/foo.py", - commit_id: str = "abc123", - html_url: str = "https://github.com/owner/repo/pull/1#discussion_r10", - created_at: str = "2024-01-01T00:00:00Z", - updated_at: str = "2024-01-01T00:00:00Z", - user: dict[str, Any] | None = None, - **extra: Any, -) -> dict[str, Any]: - return { - "id": id, - "body": body, - "path": path, - "commit_id": commit_id, - "html_url": html_url, - "created_at": created_at, - "updated_at": updated_at, - "user": user if user is not None else {"login": "reviewer"}, - **extra, - } - - -# --------------------------------------------------------------------------- -# PaginationData -# --------------------------------------------------------------------------- - -BASE = "https://api.github.com" - - -@pytest.mark.parametrize( - ("header", "expected"), - [ - (None, PaginationData()), - ("", PaginationData()), - (f'<{BASE}/page2>; rel="next"', PaginationData(next=f"{BASE}/page2")), - (f'<{BASE}/page10>; rel="last"', PaginationData(last=f"{BASE}/page10")), - ( - f'<{BASE}/page2>; rel="next", <{BASE}/page5>; rel="last"', - PaginationData(next=f"{BASE}/page2", last=f"{BASE}/page5"), - ), - ( - f'<{BASE}/page1>; rel="first", <{BASE}/page1>; rel="prev",' - f' <{BASE}/page3>; rel="next", <{BASE}/page5>; rel="last"', - PaginationData(first=f"{BASE}/page1", prev=f"{BASE}/page1", next=f"{BASE}/page3", last=f"{BASE}/page5"), - ), - ( - f'<{BASE}/page1>; rel="prev", <{BASE}/page5>; rel="last"', - PaginationData(prev=f"{BASE}/page1", last=f"{BASE}/page5"), - ), - (f'<{BASE}/page1>; rel="first"', PaginationData(first=f"{BASE}/page1")), - ], - ids=["none", "blank", "next_only", "last_only", "next_and_last", "all_links", "prev_and_last", "first_only"], -) -def test_pagination_data_from_header(header: str | None, expected: PaginationData) -> None: - p = PaginationData.from_header(header) - for f in dataclasses.fields(expected): - assert getattr(p, f.name) == getattr(expected, f.name) - - -# --------------------------------------------------------------------------- -# AsyncGitHubClient construction -# --------------------------------------------------------------------------- - - -def test_client_empty_token_raises() -> None: - with pytest.raises(ValueError, match="token"): - AsyncGitHubClient(token="") - - -def test_client_valid_token_builds_client() -> None: - client = AsyncGitHubClient(token=TOKEN) - assert isinstance(client._client, httpx.AsyncClient) - assert "Bearer ghp_test_token" in client._client.headers.get("authorization", "") # must not raise - - -async def test_client_request_headers() -> None: - captured: httpx.Headers | None = None - - def handler(request: httpx.Request) -> httpx.Response: - nonlocal captured - captured = request.headers - return json_response(workflow_run_payload()) - - client = make_client(httpx.MockTransport(handler)) - await client.get_workflow_run("o", "r", 42) - - assert captured is not None - assert captured["authorization"] == f"Bearer {TOKEN}" - assert captured["x-github-api-version"] == GITHUB_API_VERSION - assert captured["accept"] == "application/vnd.github+json" - - -# --------------------------------------------------------------------------- -# async_github_client context manager -# --------------------------------------------------------------------------- - - -async def test_context_manager_yields_client() -> None: - async with async_github_client(token=TOKEN) as client: - assert not client._client.is_closed - - -async def test_context_manager_closes_on_exit() -> None: - async with async_github_client(token=TOKEN) as client: - inner = client._client - # After exit the underlying client is closed; a new request would fail - assert inner.is_closed - - -# --------------------------------------------------------------------------- -# create_workflow_dispatch -# --------------------------------------------------------------------------- - - -async def test_create_workflow_dispatch_success() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "POST" - assert "/actions/workflows/my-workflow.yml/dispatches" in request.url.path - body = json.loads(request.content) - assert body["ref"] == "main" - assert body["return_run_details"] is True - assert "inputs" not in body - return json_response( - { - "workflow_run_id": 999, - "run_url": "https://api.github.com/repos/owner/repo/actions/runs/999", - "html_url": "https://github.com/owner/repo/actions/runs/999", - }, - headers={"x-ratelimit-remaining": "59"}, - ) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_workflow_dispatch("owner", "repo", "my-workflow.yml", "main", return_run_details=True) - assert isinstance(result, GitHubResponse) - assert isinstance(result.data, WorkflowDispatchResult) - assert result.data.workflow_run_id == 999 - assert result.headers.get("x-ratelimit-remaining") == "59" - - -async def test_create_workflow_dispatch_with_inputs() -> None: - def handler(request: httpx.Request) -> httpx.Response: - body = json.loads(request.content) - assert body["inputs"] == {"env": "prod"} - assert body["return_run_details"] is True - return json_response( - { - "workflow_run_id": 1, - "run_url": "https://api.github.com/repos/o/r/actions/runs/1", - "html_url": "https://github.com/o/r/actions/runs/1", - } - ) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_workflow_dispatch( - "o", "r", 123, "main", inputs={"env": "prod"}, return_run_details=True - ) - assert result.data.workflow_run_id == 1 - - -async def test_create_workflow_dispatch_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(422))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.create_workflow_dispatch("o", "r", "wf.yml", "main") - assert exc_info.value.response.status_code == 422 - - -async def test_create_workflow_dispatch_return_run_details_parses_response() -> None: - payload = { - "workflow_run_id": 987, - "run_url": "https://api.github.com/repos/o/r/actions/runs/987", - "html_url": "https://github.com/o/r/actions/runs/987", - } - - def handler(request: httpx.Request) -> httpx.Response: - body = json.loads(request.content) - assert body["return_run_details"] is True - return json_response(payload) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_workflow_dispatch("o", "r", "wf.yml", "main", return_run_details=True) - assert result.data.workflow_run_id == 987 - assert result.data.html_url == "https://github.com/o/r/actions/runs/987" - - -async def test_create_workflow_dispatch_omits_return_run_details_when_false() -> None: - def handler(request: httpx.Request) -> httpx.Response: - body = json.loads(request.content) - assert "return_run_details" not in body - return httpx.Response(204) - - client = make_client(httpx.MockTransport(handler)) - await client.create_workflow_dispatch("o", "r", "wf.yml", "main") - - -# --------------------------------------------------------------------------- -# get_workflow_run -# --------------------------------------------------------------------------- - - -@pytest.mark.parametrize( - ("status", "is_completed"), - [("completed", True), ("in_progress", False), ("queued", False)], - ids=["completed", "in_progress", "queued"], -) -async def test_get_workflow_run_success(status: str, is_completed: bool) -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "GET" - assert "/actions/runs/42" in request.url.path - return json_response(workflow_run_payload(status=status)) - - client = make_client(httpx.MockTransport(handler)) - result = await client.get_workflow_run("owner", "repo", 42) - assert isinstance(result.data, WorkflowRun) - assert result.data.id == 42 - assert result.data.status == status - assert result.data.is_completed is is_completed - - -async def test_get_workflow_run_headers_forwarded() -> None: - def handler(_: httpx.Request) -> httpx.Response: - return json_response(workflow_run_payload(), headers={"x-ratelimit-remaining": "50"}) - - client = make_client(httpx.MockTransport(handler)) - result = await client.get_workflow_run("o", "r", 42) - assert result.headers.get("x-ratelimit-remaining") == "50" - - -async def test_get_workflow_run_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(404))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.get_workflow_run("o", "r", 99) - assert exc_info.value.response.status_code == 404 - - -# --------------------------------------------------------------------------- -# list_workflow_run_artifacts (pagination) -# --------------------------------------------------------------------------- - - -async def test_list_workflow_run_artifacts_single_page() -> None: - artifacts = [artifact(1), artifact(2)] - - def handler(_: httpx.Request) -> httpx.Response: - return json_response({"total_count": 2, "artifacts": artifacts}) - - client = make_client(httpx.MockTransport(handler)) - pages = [] - async for page in client.list_workflow_run_artifacts("owner", "repo", 1): - pages.append(page) - - assert len(pages) == 1 - assert isinstance(pages[0].data, ArtifactsList) - assert pages[0].data.total_count == 2 - assert len(pages[0].data.artifacts) == 2 - - -async def test_list_workflow_run_artifacts_two_pages() -> None: - page1_artifacts = [artifact(1)] - page2_artifacts = [artifact(2)] - call_count = 0 - - def handler(request: httpx.Request) -> httpx.Response: - nonlocal call_count - call_count += 1 - if call_count == 1: - link = f'<{request.url.scheme}://{request.url.host}/page2>; rel="next"' - return json_response( - {"total_count": 2, "artifacts": page1_artifacts}, - headers={"link": link}, - ) - return json_response({"total_count": 2, "artifacts": page2_artifacts}) - - client = make_client(httpx.MockTransport(handler)) - pages = [] - async for page in client.list_workflow_run_artifacts("owner", "repo", 1): - pages.append(page) - - assert len(pages) == 2 - assert pages[0].data.artifacts[0].id == 1 - assert pages[1].data.artifacts[0].id == 2 - - -async def test_list_workflow_run_artifacts_pagination_stops_when_no_next() -> None: - def handler(_: httpx.Request) -> httpx.Response: - return json_response({"total_count": 1, "artifacts": [artifact(1)]}) - - client = make_client(httpx.MockTransport(handler)) - count = 0 - async for _ in client.list_workflow_run_artifacts("owner", "repo", 1): - count += 1 - assert count == 1 - - -async def test_list_workflow_run_artifacts_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(403))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - async for _ in client.list_workflow_run_artifacts("o", "r", 1): - pass - assert exc_info.value.response.status_code == 403 - - -async def test_list_workflow_run_artifacts_per_page_forwarded() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.url.params["per_page"] == "100" - return json_response({"total_count": 0, "artifacts": []}) - - client = make_client(httpx.MockTransport(handler)) - async for _ in client.list_workflow_run_artifacts("owner", "repo", 1, per_page=100): - pass - - -# --------------------------------------------------------------------------- -# list_workflow_jobs (pagination) -# --------------------------------------------------------------------------- - - -async def test_list_workflow_jobs_single_page() -> None: - jobs = [workflow_job(1), workflow_job(2, status="in_progress", conclusion=None)] - - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "GET" - assert "/actions/runs/42/jobs" in request.url.path - return json_response({"total_count": 2, "jobs": jobs}) - - client = make_client(httpx.MockTransport(handler)) - pages = [] - async for page in client.list_workflow_jobs("owner", "repo", 42): - pages.append(page) - - assert len(pages) == 1 - assert isinstance(pages[0].data, WorkflowJobsList) - assert pages[0].data.total_count == 2 - - first, second = pages[0].data.jobs - assert isinstance(first, WorkflowJob) - assert first.status is WorkflowJobStatus.COMPLETED - assert first.conclusion is WorkflowJobConclusion.SUCCESS - assert second.status is WorkflowJobStatus.IN_PROGRESS - assert second.conclusion is None - assert second.html_url == "https://github.com/owner/repo/actions/runs/42/job/2" - - # A step's status is its own StrEnum; its conclusion has no declared enum (stays str). - step = first.steps[0] - assert isinstance(step, JobStep) - assert step.status is JobStepStatus.COMPLETED - - -@pytest.mark.parametrize("status", list(WorkflowJobStatus), ids=[s.value for s in WorkflowJobStatus]) -async def test_list_workflow_jobs_parses_every_status(status: WorkflowJobStatus) -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response({"total_count": 1, "jobs": [workflow_job(1, status=status.value, conclusion=None)]}) - - client = make_client(httpx.MockTransport(handler)) - pages = [page async for page in client.list_workflow_jobs("owner", "repo", 42)] - assert pages[0].data.jobs[0].status is status - - -async def test_list_workflow_jobs_unexpected_status_raises() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response({"total_count": 1, "jobs": [workflow_job(1, status="bogus")]}) - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(ValidationError): - async for _ in client.list_workflow_jobs("owner", "repo", 42): - pass - - -async def test_list_workflow_jobs_two_pages() -> None: - call_count = 0 - - def handler(request: httpx.Request) -> httpx.Response: - nonlocal call_count - call_count += 1 - if call_count == 1: - link = f'<{request.url.scheme}://{request.url.host}/page2>; rel="next"' - return json_response({"total_count": 2, "jobs": [workflow_job(1)]}, headers={"link": link}) - return json_response({"total_count": 2, "jobs": [workflow_job(2)]}) - - client = make_client(httpx.MockTransport(handler)) - pages = [] - async for page in client.list_workflow_jobs("owner", "repo", 42): - pages.append(page) - - assert len(pages) == 2 - assert pages[0].data.jobs[0].id == 1 - assert pages[1].data.jobs[0].id == 2 - - -async def test_list_workflow_jobs_per_page_forwarded() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.url.params["per_page"] == "100" - return json_response({"total_count": 0, "jobs": []}) - - client = make_client(httpx.MockTransport(handler)) - async for _ in client.list_workflow_jobs("owner", "repo", 42, per_page=100): - pass - - -# --------------------------------------------------------------------------- -# create_issue_comment -# --------------------------------------------------------------------------- - - -async def test_create_issue_comment_success() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "POST" - assert "/issues/7/comments" in request.url.path - body = json.loads(request.content) - assert body["body"] == "LGTM" - return json_response(issue_comment_payload(body="LGTM"), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_issue_comment("owner", "repo", 7, "LGTM") - assert isinstance(result.data, IssueComment) - assert result.data.body == "LGTM" - assert isinstance(result.data.user, GitHubUser) - assert result.data.user.login == "octocat" - - -async def test_create_issue_comment_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(404))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.create_issue_comment("o", "r", 1, "hi") - assert exc_info.value.response.status_code == 404 - - -# --------------------------------------------------------------------------- -# create_pr_review_comment -# --------------------------------------------------------------------------- - - -async def test_create_pr_review_comment_success_with_position() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert "/pulls/3/comments" in request.url.path - body = json.loads(request.content) - assert body["commit_id"] == "abc123" - assert body["path"] == "src/foo.py" - assert body["position"] == 5 - assert "line" not in body - return json_response(pr_review_comment_payload(), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pr_review_comment( - "owner", "repo", 3, "Nice change", "abc123", "src/foo.py", position=5 - ) - assert isinstance(result.data, PullRequestReviewComment) - assert result.data.commit_id == "abc123" - assert isinstance(result.data.user, GitHubUser) - assert result.data.user.login == "reviewer" - - -async def test_create_pr_review_comment_success_with_line_and_side() -> None: - def handler(request: httpx.Request) -> httpx.Response: - body = json.loads(request.content) - assert body["line"] == 10 - assert body["side"] == "RIGHT" - assert "position" not in body - return json_response(pr_review_comment_payload(), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pr_review_comment("o", "r", 1, "comment", "abc123", "file.py", line=10, side="RIGHT") - assert isinstance(result.data, PullRequestReviewComment) - - -async def test_create_pr_review_comment_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(422))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.create_pr_review_comment("o", "r", 1, "body", "sha", "path") - assert exc_info.value.response.status_code == 422 - - -# --------------------------------------------------------------------------- -# create_pull_request -# --------------------------------------------------------------------------- - - -def pull_request_payload(number: int = 1, html_url: str | None = None, **extra: Any) -> dict[str, Any]: - return { - "number": number, - "html_url": html_url if html_url is not None else f"https://github.com/owner/repo/pull/{number}", - **extra, - } - - -def full_pull_request_payload( - number: int = 42, - state: str = "open", - draft: bool = True, - merged: bool = False, - locked: bool = False, - title: str = "Fix bug", - body: str = "Backport", - node_id: str = "PR_kwDOABCD123", - merge_commit_sha: str | None = None, - created_at: str = "2026-05-01T00:00:00Z", - updated_at: str = "2026-05-02T00:00:00Z", - closed_at: str | None = None, - merged_at: str | None = None, - user: dict[str, Any] | None = None, - assignees: list[dict[str, Any]] | None = None, - requested_reviewers: list[dict[str, Any]] | None = None, - labels: list[dict[str, Any]] | None = None, - head: dict[str, Any] | None = None, - base: dict[str, Any] | None = None, - **extra: Any, -) -> dict[str, Any]: - """A richer PR payload exercising sub-models (user, labels, head/base).""" - return { - "id": 9000 + number, - "number": number, - "node_id": node_id, - "url": f"https://api.github.com/repos/owner/repo/pulls/{number}", - "html_url": f"https://github.com/owner/repo/pull/{number}", - "diff_url": f"https://github.com/owner/repo/pull/{number}.diff", - "patch_url": f"https://github.com/owner/repo/pull/{number}.patch", - "state": state, - "draft": draft, - "merged": merged, - "locked": locked, - "merge_commit_sha": merge_commit_sha, - "title": title, - "body": body, - "user": user - if user is not None - else {"id": 1, "login": "octocat", "html_url": "https://github.com/octocat", "type": "User"}, - "assignees": assignees if assignees is not None else [], - "requested_reviewers": requested_reviewers - if requested_reviewers is not None - else [{"id": 2, "login": "reviewer", "type": "User"}], - "labels": labels - if labels is not None - else [ - {"id": 100, "name": "qa/skip-qa", "color": "5319e7"}, - {"id": 101, "name": "backport/7.62.x", "color": "5319e7"}, - ], - "created_at": created_at, - "updated_at": updated_at, - "closed_at": closed_at, - "merged_at": merged_at, - "head": head - if head is not None - else {"ref": "alice/fix", "sha": "1234567890abcdef00", "label": "alice:alice/fix"}, - "base": base if base is not None else {"ref": "master", "sha": "cafebabe00", "label": "owner:master"}, - **extra, - } - - -async def test_create_pull_request_success() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "POST" - assert "/repos/owner/repo/pulls" in request.url.path - body = json.loads(request.content) - assert body == { - "title": "Fix bug", - "head": "alice/fix", - "base": "master", - "body": "Fix description", - "draft": False, - } - return json_response(pull_request_payload(number=42), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pull_request("owner", "repo", "Fix bug", "alice/fix", "master", "Fix description") - assert isinstance(result.data, PullRequest) - assert result.data.number == 42 - assert result.data.html_url == "https://github.com/owner/repo/pull/42" - - -async def test_create_pull_request_draft_true_forwarded() -> None: - def handler(request: httpx.Request) -> httpx.Response: - body = json.loads(request.content) - assert body["draft"] is True - return json_response(pull_request_payload(number=7), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pull_request("o", "r", "T", "h", "b", draft=True) - assert result.data.number == 7 - - -async def test_create_pull_request_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(422))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.create_pull_request("o", "r", "T", "h", "b") - assert exc_info.value.response.status_code == 422 - - -async def test_create_pull_request_parses_full_response() -> None: - """Exercises sub-models (GitHubUser, Label, PullRequestRef) end-to-end.""" - - def handler(request: httpx.Request) -> httpx.Response: - return json_response(full_pull_request_payload(number=42), status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pull_request("owner", "repo", "Fix bug", "alice/fix", "master") - - pr = result.data - assert pr.id == 9042 - assert pr.number == 42 - assert pr.state is PullRequestState.OPEN - assert pr.draft is True - assert pr.title == "Fix bug" - - assert isinstance(pr.user, GitHubUser) - assert pr.user.login == "octocat" - - assert [label.name for label in pr.labels] == ["qa/skip-qa", "backport/7.62.x"] - assert all(isinstance(label, Label) for label in pr.labels) - - assert isinstance(pr.head, PullRequestRef) - assert pr.head.ref == "alice/fix" - assert pr.head.sha == "1234567890abcdef00" - assert isinstance(pr.base, PullRequestRef) - assert pr.base.ref == "master" - - assert [r.login for r in pr.requested_reviewers] == ["reviewer"] - assert pr.created_at == "2026-05-01T00:00:00Z" - - -async def test_create_pull_request_ignores_extra_fields() -> None: - """Unknown top-level fields in the response must not break parsing.""" - - def handler(request: httpx.Request) -> httpx.Response: - payload = full_pull_request_payload( - mergeable_state="clean", additions=42, unknown_future_field={"nested": True} - ) - return json_response(payload, status_code=201) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_pull_request("o", "r", "T", "h", "b") - assert result.data.number == 42 - - -# --------------------------------------------------------------------------- -# get_pull_request -# --------------------------------------------------------------------------- - - -@pytest.mark.parametrize("state", list(PullRequestState), ids=[s.value for s in PullRequestState]) -async def test_get_pull_request_success(state: PullRequestState) -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "GET" - assert "/repos/owner/repo/pulls/5" in request.url.path - return json_response(full_pull_request_payload(number=5, state=state.value)) - - client = make_client(httpx.MockTransport(handler)) - result = await client.get_pull_request("owner", "repo", 5) - assert isinstance(result.data, PullRequest) - assert result.data.number == 5 - assert result.data.state is state - - -async def test_get_pull_request_headers_forwarded() -> None: - def handler(_: httpx.Request) -> httpx.Response: - return json_response(full_pull_request_payload(number=5), headers={"x-ratelimit-remaining": "50"}) - - client = make_client(httpx.MockTransport(handler)) - result = await client.get_pull_request("o", "r", 5) - assert result.headers.get("x-ratelimit-remaining") == "50" - - -async def test_get_pull_request_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(404))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.get_pull_request("o", "r", 5) - assert exc_info.value.response.status_code == 404 - - -async def test_get_pull_request_unexpected_state_raises() -> None: - def handler(_: httpx.Request) -> httpx.Response: - return json_response(full_pull_request_payload(number=5, state="merged")) - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(ValidationError): - await client.get_pull_request("o", "r", 5) - - -# --------------------------------------------------------------------------- -# add_labels_to_issue -# --------------------------------------------------------------------------- - - -async def test_add_labels_to_issue_success() -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "POST" - assert "/repos/owner/repo/issues/3/labels" in request.url.path - body = json.loads(request.content) - assert body == {"labels": ["qa/skip-qa", "backport/7.62.x"]} - return json_response([{"id": 1, "name": "qa/skip-qa"}, {"id": 2, "name": "backport/7.62.x"}], status_code=200) - - client = make_client(httpx.MockTransport(handler)) - result = await client.add_labels_to_issue("owner", "repo", 3, ["qa/skip-qa", "backport/7.62.x"]) - assert [label.name for label in result.data] == ["qa/skip-qa", "backport/7.62.x"] - assert all(isinstance(label, Label) for label in result.data) - - -async def test_add_labels_to_issue_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(404))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.add_labels_to_issue("o", "r", 1, ["bug"]) - assert exc_info.value.response.status_code == 404 - - -# --------------------------------------------------------------------------- -# Lazy-loading guarantees for the `models` subpackage -# --------------------------------------------------------------------------- - - -def test_models_subpackage_loads_only_requested_submodule() -> None: - """Importing one model must not eagerly load every other model submodule. - - Runs each scenario in a clean subprocess so the import effect is observable - (the parent test process has already loaded everything for other tests). - """ - import subprocess - import sys - import textwrap - - script = textwrap.dedent( - """ - import sys - from ddev.utils.github_async.models import PullRequest # noqa: F401 - - assert 'ddev.utils.github_async.client' not in sys.modules, 'client module should not be loaded' - assert 'httpx' not in sys.modules, 'httpx should not be loaded when only models are imported' - - prefix = 'ddev.utils.github_async.models.' - loaded = sorted(name[len(prefix):] for name in sys.modules if name.startswith(prefix)) - print(','.join(loaded)) - """ - ) - result = subprocess.run([sys.executable, '-c', script], capture_output=True, text=True, check=True) - loaded = set(result.stdout.strip().split(',')) - - # `pull_request` and its two type dependencies (`user`, `label`) must load. - assert {'pull_request', 'user', 'label'} <= loaded - # Unrelated model submodules must stay unloaded. - assert 'workflow' not in loaded - assert 'comment' not in loaded - - -def test_models_subpackage_unknown_attribute_raises_attribute_error() -> None: - import ddev.utils.github_async.models as models - - with pytest.raises(AttributeError, match='no attribute'): - models.NotARealModel # noqa: B018 - - -# --------------------------------------------------------------------------- -# Custom timeout per request -# --------------------------------------------------------------------------- - - -async def test_per_request_timeout_forwarded() -> None: - """Ensure per-request timeout reaches the transport without raising.""" - - def handler(request: httpx.Request) -> httpx.Response: - return json_response(workflow_run_payload()) - - client = AsyncGitHubClient(token=TOKEN, default_timeout=5.0, transport=httpx.MockTransport(handler)) - result = await client.get_workflow_run("o", "r", 42, timeout=2.0) - assert result.data.id == 42 - - -# --------------------------------------------------------------------------- -# Rate limiter wiring in AsyncGitHubClient -# --------------------------------------------------------------------------- - - -async def test_client_request_without_rate_limiter_goes_through() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response(workflow_run_payload()) - - async with async_github_client(token=TOKEN, transport=httpx.MockTransport(handler)) as client: - result = await client.get_workflow_run("o", "r", 42) - - assert result.data.id == 42 - - -async def test_client_request_with_rate_limiter_consumes_token() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response(workflow_run_payload()) - - real_limiter = AsyncLimiter(max_rate=1, time_period=1000) - events: list[RateLimitEvent] = [] - rate_limiter = InstrumentedAsyncLimiter(real_limiter, on_event=events.append) - - async with async_github_client( - token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) - ) as client: - result = await client.get_workflow_run("o", "r", 42) - - assert result.data.id == 42 - bucket_events = [event for event in events if isinstance(event, BucketEvent)] - assert bucket_events == [BucketEvent(throttled=False, name="")] - assert not real_limiter.has_capacity() - - -async def test_client_request_throttled_when_bucket_exhausted() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response(workflow_run_payload()) - - # A short time_period lets the second, throttled acquire actually complete instead of - # blocking forever, so its BucketEvent fires. - real_limiter = AsyncLimiter(max_rate=1, time_period=0.1) - events: list[RateLimitEvent] = [] - rate_limiter = InstrumentedAsyncLimiter(real_limiter, on_event=events.append) - - async with async_github_client( - token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) - ) as client: - await client.get_workflow_run("o", "r", 42) # drains the single token - - async with async_github_client( - token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) - ) as client: - await client.get_workflow_run("o", "r", 42) # blocks until the 0.1s period refills it - - bucket_events = [event for event in events if isinstance(event, BucketEvent)] - assert [event.throttled for event in bucket_events] == [False, True] - - -# --------------------------------------------------------------------------- -# github_rate_limit_snapshot -# --------------------------------------------------------------------------- - - -@pytest.mark.parametrize( - ("headers", "expected"), - [ - ( - { - "x-ratelimit-limit": "5000", - "x-ratelimit-remaining": "4321", - "x-ratelimit-reset": "1700000000", - "retry-after": "30", - }, - BudgetSnapshot(limit=5000, remaining=4321, reset_at=1700000000.0, retry_after=30.0), - ), - ( - {"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4999"}, - BudgetSnapshot(limit=5000, remaining=4999), - ), - ({"x-ratelimit-limit": "5000", "retry-after": "not-a-number"}, BudgetSnapshot(limit=5000)), - ({"x-ratelimit-limit": "not-a-number"}, None), - ({"content-type": "application/json"}, None), - ], - ids=["all_present", "partial_primary", "non_integer_retry_after", "all_unparseable", "no_ratelimit_headers"], -) -def test_github_rate_limit_snapshot(headers: dict[str, str], expected: BudgetSnapshot | None) -> None: - assert github_rate_limit_snapshot(httpx.Headers(headers)) == expected - - -async def test_client_request_observes_rate_limit_headers_into_governor() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response( - workflow_run_payload(), - headers={"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "4999", "x-ratelimit-reset": "1700000000"}, - ) - - governor = BudgetGovernor() - rate_limiter = InstrumentedAsyncLimiter(AsyncLimiter(max_rate=1, time_period=1000), budget_governor=governor) - - async with async_github_client( - token=TOKEN, rate_limiter=rate_limiter, transport=httpx.MockTransport(handler) - ) as client: - await client.get_workflow_run("o", "r", 42) - - assert governor.budget.limit == 5000 - assert governor.budget.remaining == 4999 - assert governor.budget.reset_at == 1700000000.0 - - -# --------------------------------------------------------------------------- -# create_check_run / update_check_run -# --------------------------------------------------------------------------- - - -def check_run_payload( - id: int = 1, - name: str = "ck", - status: str = "in_progress", - head_sha: str = "abc", - conclusion: str | None = None, - html_url: str = "https://github.com/o/r/check-runs/1", - **extra: Any, -) -> dict[str, Any]: - return { - "id": id, - "name": name, - "status": status, - "head_sha": head_sha, - "conclusion": conclusion, - "html_url": html_url, - **extra, - } - - -@pytest.mark.asyncio -async def test_create_check_run_success() -> None: - captured: dict[str, Any] = {} - - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "POST" - assert "/repos/o/r/check-runs" in request.url.path - captured.update(json.loads(request.content)) - return json_response(check_run_payload(name=captured["name"], head_sha=captured["head_sha"])) - - client = make_client(httpx.MockTransport(handler)) - result = await client.create_check_run("o", "r", name="ck", head_sha="abc", status="in_progress") - assert isinstance(result.data, CheckRun) - assert result.data.id == 1 - assert result.data.status is CheckRunStatus.IN_PROGRESS - assert captured == {"name": "ck", "head_sha": "abc", "status": "in_progress"} - - -@pytest.mark.asyncio -async def test_create_check_run_with_optional_fields() -> None: - captured: dict[str, Any] = {} - - def handler(request: httpx.Request) -> httpx.Response: - captured.update(json.loads(request.content)) - return json_response(check_run_payload()) - - client = make_client(httpx.MockTransport(handler)) - await client.create_check_run( - "o", - "r", - name="ck", - head_sha="abc", - status="in_progress", - details_url="https://x", - output={"title": "t", "summary": "s"}, - ) - assert captured["details_url"] == "https://x" - assert captured["output"] == {"title": "t", "summary": "s"} - - -@pytest.mark.asyncio -async def test_create_check_run_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(422))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.create_check_run("o", "r", name="ck", head_sha="abc", status="in_progress") - assert exc_info.value.response.status_code == 422 - - -# One case per non-completed status (conclusion is null until completed), plus every -# conclusion (incl. the GitHub-only ``stale``) paired with ``completed``. -CHECK_RUN_RESULT_CASES = [ - *[ - pytest.param(status, None, id=status.value) - for status in CheckRunStatus - if status is not CheckRunStatus.COMPLETED - ], - *[ - pytest.param(CheckRunStatus.COMPLETED, conclusion, id=f"completed-{conclusion.value}") - for conclusion in CheckRunConclusion - ], -] - - -@pytest.mark.asyncio -@pytest.mark.parametrize(("status", "conclusion"), CHECK_RUN_RESULT_CASES) -async def test_update_check_run_success(status: CheckRunStatus, conclusion: CheckRunConclusion | None) -> None: - def handler(request: httpx.Request) -> httpx.Response: - assert request.method == "PATCH" - assert "/repos/o/r/check-runs/77" in request.url.path - return json_response( - check_run_payload(id=77, status=status.value, conclusion=conclusion.value if conclusion else None) - ) - - client = make_client(httpx.MockTransport(handler)) - result = await client.update_check_run("o", "r", 77, status="in_progress") - assert result.data.status is status - assert result.data.conclusion is conclusion - - -@pytest.mark.asyncio -async def test_update_check_run_omits_unset_fields() -> None: - captured: dict[str, Any] = {} - - def handler(request: httpx.Request) -> httpx.Response: - captured.update(json.loads(request.content)) - return json_response(check_run_payload(id=77)) - - client = make_client(httpx.MockTransport(handler)) - await client.update_check_run("o", "r", 77, conclusion="failure") - assert captured == {"conclusion": "failure"} - - -@pytest.mark.asyncio -async def test_update_check_run_requires_conclusion_when_completed() -> None: - requested = False - - def handler(request: httpx.Request) -> httpx.Response: - nonlocal requested - requested = True - return json_response(check_run_payload()) - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(ValueError, match="conclusion is required"): - await client.update_check_run("o", "r", 77, status="completed") - assert requested is False - - -@pytest.mark.asyncio -async def test_update_check_run_http_error_raises() -> None: - client = make_client(httpx.MockTransport(lambda r: httpx.Response(404))) - with pytest.raises(httpx.HTTPStatusError) as exc_info: - await client.update_check_run("o", "r", 77, status="in_progress") - assert exc_info.value.response.status_code == 404 - - -@pytest.mark.asyncio -async def test_update_check_run_unexpected_status_raises() -> None: - def handler(request: httpx.Request) -> httpx.Response: - return json_response(check_run_payload(id=77, status="bogus")) - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(ValidationError): - await client.update_check_run("o", "r", 77, status="in_progress") - - -# --------------------------------------------------------------------------- -# download_artifact -# --------------------------------------------------------------------------- - - -def make_zip(members: dict[str, bytes]) -> bytes: - buf = io.BytesIO() - with zipfile.ZipFile(buf, "w") as zf: - for name, content in members.items(): - zf.writestr(name, content) - return buf.getvalue() - - -@pytest.mark.asyncio -async def test_download_artifact_token_not_leaked_to_redirect_target(monkeypatch, tmp_path) -> None: - captured_signed_headers: dict[str, str] = {} - - def github_handler(request: httpx.Request) -> httpx.Response: - assert request.headers["authorization"].startswith("Bearer ") - return httpx.Response(302, headers={"location": "https://signed.example/zip"}) - - def signed_handler(request: httpx.Request) -> httpx.Response: - captured_signed_headers.update({k.lower(): v for k, v in request.headers.items()}) - return httpx.Response(200, content=make_zip({"hello.txt": b"hi"})) - - real_async_client = httpx.AsyncClient - - def fake_async_client(*args: Any, **kwargs: Any) -> httpx.AsyncClient: - if kwargs.get("transport") is None: - kwargs["transport"] = httpx.MockTransport(signed_handler) - return real_async_client(*args, **kwargs) - - monkeypatch.setattr("ddev.utils.github_async.client.httpx.AsyncClient", fake_async_client) - - client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - assert "authorization" not in captured_signed_headers - assert (tmp_path / "out" / "hello.txt").read_bytes() == b"hi" - - -@pytest.mark.asyncio -async def test_download_artifact_non_302_raises(tmp_path) -> None: - def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(200, content=b"not a redirect") - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(httpx.HTTPError, match="Expected 302"): - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - -@pytest.mark.asyncio -async def test_download_artifact_signed_url_error_raises(monkeypatch, tmp_path) -> None: - def github_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(302, headers={"location": "https://signed.example/zip"}) - - def signed_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(403, content=b"expired") - - real_async_client = httpx.AsyncClient - - def fake_async_client(*args: Any, **kwargs: Any) -> httpx.AsyncClient: - if kwargs.get("transport") is None: - kwargs["transport"] = httpx.MockTransport(signed_handler) - return real_async_client(*args, **kwargs) - - monkeypatch.setattr("ddev.utils.github_async.client.httpx.AsyncClient", fake_async_client) - - client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) - with pytest.raises(httpx.HTTPStatusError): - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - -@pytest.mark.asyncio -async def test_download_artifact_missing_location_header_raises(tmp_path) -> None: - def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(302) - - client = make_client(httpx.MockTransport(handler)) - with pytest.raises(httpx.HTTPError, match="Missing Location"): - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - -@pytest.mark.parametrize( - "malicious_member", - [ - pytest.param("../escape.txt", id="parent-traversal"), - pytest.param("/etc/passwd", id="absolute-path"), - ], -) -@pytest.mark.asyncio -async def test_download_artifact_zip_slip_rejected(monkeypatch, tmp_path, malicious_member: str) -> None: - def github_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(302, headers={"location": "https://signed.example/zip"}) - - def signed_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(200, content=make_zip({malicious_member: b"pwn"})) - - real_async_client = httpx.AsyncClient - - def fake_async_client(*args: Any, **kwargs: Any) -> httpx.AsyncClient: - if kwargs.get("transport") is None: - kwargs["transport"] = httpx.MockTransport(signed_handler) - return real_async_client(*args, **kwargs) - - monkeypatch.setattr("ddev.utils.github_async.client.httpx.AsyncClient", fake_async_client) - - client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) - dest = tmp_path / "out" - with pytest.raises(ValueError, match="(?i)zip-slip"): - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", dest) - - # Nothing was extracted before the guard fired. - assert list(dest.rglob("*")) == [] - - -def patch_signed_download(monkeypatch, handler: Any) -> None: - """Route the anonymous signed-URL download through a mock transport.""" - real_async_client = httpx.AsyncClient - - def fake_async_client(*args: Any, **kwargs: Any) -> httpx.AsyncClient: - if kwargs.get("transport") is None: - kwargs["transport"] = httpx.MockTransport(handler) - return real_async_client(*args, **kwargs) - - monkeypatch.setattr("ddev.utils.github_async.client.httpx.AsyncClient", fake_async_client) - - -@pytest.mark.asyncio -async def test_download_artifact_server_error_propagates(monkeypatch, tmp_path) -> None: - """A failed signed-URL download propagates as httpx.HTTPStatusError (no retries).""" - - def github_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(302, headers={"location": "https://signed.example/zip"}) - - def signed_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(503, content=b"unavailable") - - patch_signed_download(monkeypatch, signed_handler) - client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) - with pytest.raises(httpx.HTTPStatusError): - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - -# --------------------------------------------------------------------------- -# Default rate-limit protection + rate-limit-aware retry -# --------------------------------------------------------------------------- - - -class FakeClock: - """Injectable, manually-advanceable clock for deterministic governor-driven retries.""" - - def __init__(self, start: float = 1000.0) -> None: - self.current = start - - def __call__(self) -> float: - return self.current - - def advance(self, seconds: float) -> None: - self.current += seconds - - -def recording_transport(items: list[httpx.Response | Exception]) -> tuple[httpx.MockTransport, list[httpx.Request]]: - """MockTransport that returns/raises *items* in order (last item repeats) and records each request.""" - calls: list[httpx.Request] = [] - - def handler(request: httpx.Request) -> httpx.Response: - item = items[min(len(calls), len(items) - 1)] - calls.append(request) - if isinstance(item, Exception): - raise item - return item - - return httpx.MockTransport(handler), calls - - -def governed_client( - clock: FakeClock, - transport: httpx.MockTransport, - on_event: Any = None, - max_rate_limit_retries: int = 2, -) -> AsyncGitHubClient: - """Client whose governor runs on *clock*, so retry waits are deterministic under a fake sleep.""" - governor = BudgetGovernor(now=clock, on_event=on_event) - limiter = InstrumentedAsyncLimiter( - AsyncLimiter(max_rate=5000, time_period=RATE_LIMIT_TIME_PERIOD), - on_event=on_event, - budget_governor=governor, - name="github", - ) - return AsyncGitHubClient( - token=TOKEN, rate_limiter=limiter, transport=transport, max_rate_limit_retries=max_rate_limit_retries - ) - - -def advance_clock_on_sleep(clock: FakeClock, monkeypatch: pytest.MonkeyPatch) -> None: - """Make asyncio.sleep advance the fake clock instead of blocking, so governor waits are instant.""" - - async def fake_sleep(delay: float) -> None: - clock.advance(delay) - - monkeypatch.setattr(asyncio, "sleep", fake_sleep) - - -async def test_default_rate_limiter_is_constructed_and_observes_403() -> None: - """rate_limiter=None builds a limiter with a governor that observes a 403's retry-after.""" - transport, calls = recording_transport([httpx.Response(403, headers={"retry-after": "30"})]) - client = AsyncGitHubClient(token=TOKEN, transport=transport, max_rate_limit_retries=0) - - assert client._rate_limiter is not None - governor = client._rate_limiter.budget_governor - assert governor is not None - - with pytest.raises(httpx.HTTPStatusError): - await client._request("GET", "/x") - - # The 403's retry-after was observed (before raise_for_status), arming the shared pause. - assert governor.pause_until - time.time() == pytest.approx(31.0, abs=2.0) - assert len(calls) == 1 # retries disabled: one call, no wait - - -async def test_retry_on_secondary_limit_returns_success(monkeypatch: pytest.MonkeyPatch) -> None: - """A 403 with retry-after then a 200 is retried once and the wait goes through the governor.""" - clock = FakeClock() - advance_clock_on_sleep(clock, monkeypatch) - events: list[RateLimitEvent] = [] - transport, calls = recording_transport([httpx.Response(403, headers={"retry-after": "5"}), httpx.Response(200)]) - client = governed_client(clock, transport, on_event=events.append) - - response = await client._request("GET", "/x") - - assert response.status_code == 200 - assert len(calls) == 2 - secondary_index = next(i for i, e in enumerate(events) if isinstance(e, SecondaryLimitEvent)) - pacing_index = next( - i for i, e in enumerate(events) if isinstance(e, PacingEvent) and e.reason is PacingReason.SECONDARY_LIMIT - ) - assert secondary_index < pacing_index - - -async def test_retry_on_primary_exhaustion_waits_until_reset(monkeypatch: pytest.MonkeyPatch) -> None: - """A 403 with x-ratelimit-remaining=0 is retried, and the retry waits until the window reset.""" - clock = FakeClock() - advance_clock_on_sleep(clock, monkeypatch) - events: list[RateLimitEvent] = [] - reset_at = clock.current + 30 - transport, calls = recording_transport( - [ - httpx.Response( - 403, - headers={"x-ratelimit-limit": "5000", "x-ratelimit-remaining": "0", "x-ratelimit-reset": str(reset_at)}, - ), - httpx.Response(200), - ] - ) - client = governed_client(clock, transport, on_event=events.append) - - response = await client._request("GET", "/x") - - assert response.status_code == 200 - assert len(calls) == 2 - governor = client._rate_limiter.budget_governor - assert clock.current == pytest.approx(reset_at + governor.buffer_seconds) - assert any(isinstance(e, PacingEvent) and e.reason is PacingReason.EXHAUSTED for e in events) - - -async def test_no_retry_on_permission_denied_403() -> None: - """A 403 with no retry-after and nonzero remaining is a permission denial: raise on first attempt.""" - transport, calls = recording_transport([httpx.Response(403, headers={"x-ratelimit-remaining": "5"})]) - client = AsyncGitHubClient(token=TOKEN, transport=transport) - - with pytest.raises(httpx.HTTPStatusError): - await client._request("GET", "/x") - - assert len(calls) == 1 - - -async def test_no_retry_on_transport_error() -> None: - """A transport error is never retried (the action may have executed); it propagates immediately.""" - transport, calls = recording_transport([httpx.ConnectError("boom")]) - client = AsyncGitHubClient(token=TOKEN, transport=transport) - - with pytest.raises(httpx.ConnectError): - await client._request("GET", "/x") - - assert len(calls) == 1 - - -async def test_retries_exhausted_raises_after_max(monkeypatch: pytest.MonkeyPatch) -> None: - """Two consecutive rate-limit responses with max_rate_limit_retries=1 raise after exactly two calls.""" - clock = FakeClock() - advance_clock_on_sleep(clock, monkeypatch) - transport, calls = recording_transport( - [httpx.Response(403, headers={"retry-after": "5"}), httpx.Response(403, headers={"retry-after": "5"})] - ) - client = governed_client(clock, transport, max_rate_limit_retries=1) - - with pytest.raises(httpx.HTTPStatusError): - await client._request("GET", "/x") - - assert len(calls) == 2 - - -async def test_download_redirect_302_is_not_retried(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: - """The artifact 302 redirect is not a rate-limit response, so it resolves without any retry.""" - github_calls: list[httpx.Request] = [] - - def github_handler(request: httpx.Request) -> httpx.Response: - github_calls.append(request) - return httpx.Response(302, headers={"location": "https://signed.example/zip"}) - - def signed_handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(200, content=make_zip({"hello.txt": b"hi"})) - - patch_signed_download(monkeypatch, signed_handler) - client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler)) - - await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out") - - assert len(github_calls) == 1 - - -async def test_pagination_retries_only_the_rate_limited_page(monkeypatch: pytest.MonkeyPatch) -> None: - """A rate-limited page is retried in place; the iterator then continues to the next page.""" - clock = FakeClock() - advance_clock_on_sleep(clock, monkeypatch) - transport, calls = recording_transport( - [ - httpx.Response(200, json={"page": 1}, headers={"link": '; rel="next"'}), - httpx.Response(403, headers={"retry-after": "5"}), - httpx.Response(200, json={"page": 2}), - ] - ) - client = governed_client(clock, transport) - - pages = [response async for response in client._paginated_request("GET", "/start")] - - assert [page.status_code for page in pages] == [200, 200] - assert len(calls) == 3 # page 1, page 2 (rate-limited), page 2 (retry) diff --git a/ddev/tests/utils/test_github_async_defaults.py b/ddev/tests/utils/test_github_async_defaults.py deleted file mode 100644 index 6a50e8992170b..0000000000000 --- a/ddev/tests/utils/test_github_async_defaults.py +++ /dev/null @@ -1,67 +0,0 @@ -"""Tests for the default GitHub rate-limit protection.""" - -from __future__ import annotations - -import dataclasses -import logging - -import pytest - -from ddev.utils.github_async.defaults import default_github_rate_limiter, log_rate_limit_events -from ddev.utils.rate_limiting import ( - BudgetSnapshot, - PacingEvent, - PacingReason, - RateLimitEvent, - SecondaryLimitEvent, -) - -LOGGER_NAME = "ddev.utils.github_async.defaults" - - -async def test_default_github_rate_limiter_wires_callback_into_both_slots() -> None: - """The factory must wire the callback into both the bucket and the governor slots.""" - seen: list[RateLimitEvent] = [] - limiter = default_github_rate_limiter(on_event=seen.append) - - await limiter.__aenter__() # bucket acquire -> BucketEvent (limiter slot) - limiter.observe(BudgetSnapshot(limit=100, remaining=10, reset_at=2000.0)) # observe -> BudgetEvent (governor slot) - - kinds = {type(event).__name__ for event in seen} - assert "BucketEvent" in kinds # would be missing if the limiter slot were unwired - assert "BudgetEvent" in kinds # would be missing if the governor slot were unwired - - -@pytest.mark.parametrize( - ("event", "expected_level"), - [ - pytest.param( - SecondaryLimitEvent(retry_after_seconds=5.0, pause_seconds=6.0), logging.WARNING, id="secondary_limit" - ), - pytest.param(PacingEvent(wait_seconds=0.0, reason=PacingReason.NONE), logging.DEBUG, id="healthy_pacing"), - pytest.param(PacingEvent(wait_seconds=3.0, reason=PacingReason.RATIONING), logging.INFO, id="rationing"), - pytest.param(PacingEvent(wait_seconds=9.0, reason=PacingReason.EXHAUSTED), logging.WARNING, id="exhausted"), - pytest.param(PacingEvent(wait_seconds=9.0, reason=PacingReason.ABANDONED), logging.ERROR, id="abandoned"), - ], -) -def test_log_rate_limit_events_level_mapping( - caplog: pytest.LogCaptureFixture, event: RateLimitEvent, expected_level: int -) -> None: - with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): - log_rate_limit_events()(event) - - assert caplog.records - assert caplog.records[-1].levelno == expected_level - - -def test_log_rate_limit_events_unknown_event_does_not_raise(caplog: pytest.LogCaptureFixture) -> None: - """A future event type must fall through to the DEBUG catch-all, never raise.""" - - @dataclasses.dataclass(frozen=True) - class FutureEvent: - type: str = "future" - - with caplog.at_level(logging.DEBUG, logger=LOGGER_NAME): - log_rate_limit_events()(FutureEvent()) # type: ignore[arg-type] - - assert caplog.records[-1].levelno == logging.DEBUG diff --git a/ddev/tests/utils/test_rate_limiting.py b/ddev/tests/utils/test_rate_limiting.py index 315f944a47f8f..5bc6c2c49a187 100644 --- a/ddev/tests/utils/test_rate_limiting.py +++ b/ddev/tests/utils/test_rate_limiting.py @@ -26,19 +26,7 @@ SecondaryLimitEvent, ) from tests.helpers.assertions import assert_blocks - - -class FakeClock: - """Injectable, manually-advanceable clock for deterministic governor tests.""" - - def __init__(self, start: float = 1000.0) -> None: - self.current = start - - def __call__(self) -> float: - return self.current - - def advance(self, seconds: float) -> None: - self.current += seconds +from tests.helpers.clock import FakeClock def make_snapshot( diff --git a/incident_io/assets/account_config.json b/incident_io/assets/account_config.json index d233d38edfea8..e398b70764cf9 100644 --- a/incident_io/assets/account_config.json +++ b/incident_io/assets/account_config.json @@ -4,7 +4,7 @@ "auth_name": "API Key", "bearer_token": { "password_setting": { - "key": "incident_io_api_key", + "key": "api_token", "label": "API key", "help": "incident.io API Key", "required": true, diff --git a/snmp/assets/configuration/spec.yaml b/snmp/assets/configuration/spec.yaml index b08bc58af62df..dfa8ae6999d4c 100644 --- a/snmp/assets/configuration/spec.yaml +++ b/snmp/assets/configuration/spec.yaml @@ -96,6 +96,15 @@ files: definition_file: cisco-asr.yaml hidden: true fleet_configurable: false + - name: use_remote_config_profiles + description: | + Whether to apply device profiles from the SNMP Profile Manager, managed through Remote Configuration. + When enabled, any locally defined profiles are ignored in favor of the remotely-managed profiles. + value: + type: boolean + example: true + display_default: false + fleet_configurable: true - name: refresh_oids_cache_interval legacy: true description: | diff --git a/snmp/changelog.d/24528.added b/snmp/changelog.d/24528.added new file mode 100644 index 0000000000000..2baaa076d3f5e --- /dev/null +++ b/snmp/changelog.d/24528.added @@ -0,0 +1 @@ +Add the `use_remote_config_profiles` option to the SNMP configuration spec so it is accepted by config validation and Remote Configuration. diff --git a/snmp/datadog_checks/snmp/data/conf.yaml.example b/snmp/datadog_checks/snmp/data/conf.yaml.example index 4c434d453ce44..68ea5f4aec714 100644 --- a/snmp/datadog_checks/snmp/data/conf.yaml.example +++ b/snmp/datadog_checks/snmp/data/conf.yaml.example @@ -55,6 +55,12 @@ init_config: # metric_tags: # - TCP + ## @param use_remote_config_profiles - boolean - optional - default: false + ## Whether to apply device profiles from the SNMP Profile Manager, managed through Remote Configuration. + ## When enabled, any locally defined profiles are ignored in favor of the remotely-managed profiles. + # + # use_remote_config_profiles: true + ## @param refresh_oids_cache_interval - integer - optional - default: 0 ## Note: Beta feature, only available using python SNMP integration. ## Set this option to enable caching of OIDs. The value is the number of seconds before the