[pull] master from DataDog:master#654
Merged
Merged
Conversation
* Extract shared async-test helpers (pure moves) - Move FakeClock and advance_clock_on_sleep to tests/helpers/clock.py; they were duplicated between the client and rate-limiting suites and would drift. Both suites now import from there; the rate-limiting slept fixture keeps its local recording behavior on top of the shared clock. - Point the download tests (token_not_leaked, signed_url_error, zip_slip) at the existing patch_signed_download helper instead of copy-pasting its monkeypatch block. - Strip explicit @pytest.mark.asyncio from the check-run and download sections; asyncio auto mode is the convention. * Delete, merge, and relocate async-client tests that cannot fail for client reasons Coverage over ddev/utils/github_async is unchanged (430/430 executed lines). Deleted outright: - test_per_request_timeout_forwarded: its handler never inspected the timeout, so it could not fail if forwarding broke. Replaced with a parametrized test that asserts request.extensions["timeout"] for a per-request override and the constructor default. - test_client_valid_token_builds_client: poked _client internals; the request-headers test verifies the same contract through a real request. - test_create_workflow_dispatch_return_run_details_parses_response: re-asserts what the success test already asserts with different numbers. - test_list_workflow_run_artifacts_pagination_stops_when_no_next: weaker copy of the single-page test. - test_client_request_without_rate_limiter_goes_through: premise removed by the default-limiter change; body was a generic success check. - test_client_request_throttled_when_bucket_exhausted: re-tests InstrumentedAsyncLimiter throttled-event semantics owned by the rate-limiting suite. Merged: - signed-URL 403 and 503 propagation into one test parametrized over status code. - the two two-page pagination tests into one (link-following lives in _paginated_request). Collapsed enum matrices (a request-constant/response-varying matrix over a StrEnum field verifies Pydantic, not the client; the client boundary is one valid value parses, a bogus value raises, and the bogus tests remain): - CHECK_RUN_RESULT_CASES to one representative (completed + success). - test_list_workflow_jobs_parses_every_status removed (single-page parses two statuses). - test_get_pull_request_success to one state. Relocated to tests/utils/github_async/test_models.py as direct model_validate calls (no transport): full-response sub-model parsing, extra-field tolerance, subpackage unknown-attribute, and the lazy-import subprocess test. Fixed test_default_rate_limiter_is_constructed_and_observes_403 to drop time.time() coupling; exact pause arithmetic is owned by the clocked tests. Added one end-to-end retry test through the public endpoint surface (get_workflow_run retries a rate-limit response and returns the parsed model). * Replace per-endpoint error/header tests with a registry-driven pair Add ENDPOINT_CALLS, a registry of every public endpoint method (minimal valid args plus a valid response factory), and drive two parametrized tests off it: - test_endpoint_http_error_raises: a 422 transport must surface as HTTPStatusError. - test_endpoint_forwards_response_headers: response headers must land on GitHubResponse.headers. This removes eleven near-identical per-endpoint *_http_error_raises and *_headers_forwarded tests. New endpoint methods get error and header coverage by registering once, instead of copying a test each. download_artifact is excluded (it does not return a GitHubResponse) and keeps its own tests. Coverage over ddev/utils/github_async is unchanged (430/430). * Split the async-client test file into a package Break the monolithic tests/utils/test_github_async.py into tests/utils/github_async/: - conftest.py: shared fixtures, payload factories, recording_transport, governed_client, make_zip, patch_signed_download, and the ENDPOINT_CALLS registry. - test_client_core.py: construction, auth headers, context manager, timeout forwarding, PaginationData, and pagination mechanics. - test_endpoints.py: per-endpoint success tests plus the registry-driven cross-cutting pair. - test_models.py: model parsing contracts (already relocated earlier). - test_download_artifact.py: redirect, token non-leak, zip-slip, signed-URL failures. - test_rate_limiting.py: snapshot parsing, governor wiring, default construction, the retry suite, and the default-limiter logging tests folded in from test_github_async_defaults.py. Coverage over ddev/utils/github_async is unchanged (430/430 executed lines). * Add test guidelines for the async GitHub client suite Encode the rules the refactor established so the suite does not regrow: the "can this fail for a client-code reason" gate, layer ownership (models via model_validate, limiter/governor in the rate-limiting suite, no enum exhaustiveness), the ENDPOINT_CALLS registry for cross-cutting coverage, one success test per endpoint, determinism rules, and the requirement that assertions be able to fail. * Move shared test helpers out of conftest and document test-helper placement Importing from a conftest.py gives it a second module identity and can double-register its fixtures. Rename the package conftest.py to helpers.py (a plain module) and repoint every import; the package needs no conftest since it defines no fixtures. Add ddev/tests/AGENTS.md documenting the conftest runtime rule (nothing imports from conftest except fixture types under TYPE_CHECKING) and the helper scope/placement scheme (local helpers.py, subtree helpers/, global tests/helpers/; promote never copy; imports point upward). This lives under ddev/tests rather than the repo-root AGENTS.md because the scope scheme is ddev-specific. * Split test helpers by layer so model tests carry no client imports Addresses the Codex review: test_models.py imported a payload factory from the mixed helpers.py, which eagerly pulled in httpx, AsyncGitHubClient, and the rate-limiting stack, contradicting the "no transport or client" boundary the model suite is supposed to hold. Split the shared helpers by layer: - payloads.py: pure dict payload factories, stdlib-only imports. This is all test_models.py needs. - helpers.py: client-layer material (json_response, make_client, recording_transport, governed_client, patch_signed_download, make_zip, first_page, EndpointCase, ENDPOINT_CALLS), importing response bodies from payloads, never the reverse. Repoint every test's payload-factory imports at payloads.py. Document the rule in the package AGENTS.md layer-ownership section: helper modules obey the same layer boundaries as tests, and a model test needing something from helpers.py means the helper is misplaced or the test is not a model test.
* Support per-invocation env vars for e2e Agent containers and a new --env option on ddev env agent check to set an environment variable scoped to a single agent check invocation (via docker exec -e) rather than the whole Agent container. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * tests * error * validate * validate * Return env var dict directly from --env validation callback Address review comment: avoid re-parsing the KEY=VALUE entries a second time after the callback has already validated them. * Parametrize malformed --env value tests Address review comment: combine the two near-identical "Invalid value for '--env'" test cases via pytest.mark.parametrize. * fixup --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Add use_remote_config_profiles to SNMP config spec The `use_remote_config_profiles` option is shipped and functional in the Agent's autodiscovery template (cmd/agent/dist/conf.d/snmp.d/auto_conf.yaml) and accepted by the corecheck, but it was never present in this integration's spec.yaml. Because the generated config schema had no such property, Remote Configuration rejected it with "Property use_remote_config_profiles is not allowed" (AGENT-16536). Add it under init_config and regenerate conf.yaml.example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add changelog entry Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )