Move 36 check test suites onto the agnostic HTTP doubles - #24831
Draft
mwdd146980 wants to merge 1 commit into
Draft
Move 36 check test suites onto the agnostic HTTP doubles#24831mwdd146980 wants to merge 1 commit into
mwdd146980 wants to merge 1 commit into
Conversation
These suites patched requests.Session and built requests.Response objects directly, so they were written against the HTTP library rather than against the client contract. Route them through the mock_http, mock_openmetrics_http, and mock_prometheus_http doubles and MockHTTPResponse, so a backend change cannot break them. Harness HTTP in dd_environment is left on requests. It stands up the test environment rather than intercepting the check, so it cannot break on a backend flip.
|
Contributor
Validation ReportAll 21 validations passed. Show details
|
evalya-impact-summaryevalya impact analysis |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
Moves 36 check test suites off the
requestslibrary and onto the library-agnostic HTTP test doubles that #24815 adds todatadog_checks_dev.The suites patched
requests.Sessionand builtrequests.Responseobjects, so they were written against the HTTP library rather than against the client contract. They now read through the sealed doubles instead:mock_httpreplaces 66requests.Sessionpatches.mock_openmetrics_httpandmock_prometheus_httpcover the OpenMetrics and Prometheus scrapers, 59 call sites.MockHTTPResponsereplaces the requests-basedMockResponse, 44 call sites.nginx,riak, andazure_iot_edgeassert on the agnostic exception types rather than onrequests.exceptions.No check source changes, so no changelog entry and no base-package floor bump.
Harness HTTP stays on
requests. The calls inhaproxy'sdd_environmentstand up the test environment rather than intercepting the check, so they cannot break on a backend change.Motivation
This continues confining
requeststo the base check. Once a suite reads through the doubles it depends on the client contract rather than on the backend, which is what makes the backend replaceable without touching every test.Known failure: minimum-base-package
The
test-minimum-base-packagejobs fail on this PR, and will keep failing untildatadog-checks-base38.1.0 is published. Those jobs runddev test --compat, which installs each integration's declared floor of>=37.33.0. The doubles and the agnostic exception modules exist only from 38.1.0 onward, so the conftests cannot import them. Reproduced locally:Note that #24815 does not surface this. A PR touching
datadog_checks_base/datadog_checks/ordatadog_checks_dev/datadog_checks/dev/*.pymatchesSKIPPED_PATTERNinci_matrix.pyand is routed to thePR Allworkflow, which carries no minimum-base-package job. This PR touches only test files, so it gets the standard matrix and the jobs expand.Resolving it needs a floor bump to
>=38.1.0across the 36 integrations, which can only land once that version is on PyPI. It is deliberately left out here so this diff stays test-only.Worth flagging separately: the same mechanism hides a wider problem on #24815.
ddev test --compat karpenterpasses on cleanmasterand fails on that branch withModuleNotFoundError: No module named 'datadog_checks.base.utils.http_exceptions', raised fromMockHTTPResponse.raise_for_status. 69 integrations consume a fixture that now yieldsMockHTTPResponse, andkarpenteris touched by neither PR.Verification
All 36 suites were run locally. 429 passed, and every non-zero exit was reproduced away from this change.
docker composeor connection refused under Colimaetcd,impala,ray,riak,scylla,silk,temporalmasterciliumciliumreports the identical 2 failures on cleanorigin/master,2 failed, 30 passed, 5 skipped, and both are markedskip_on_ci.ddev test --lintis clean across all 36.The 63 files are byte-identical to #22676 at
bf01ee0c1f, verified withgit diff.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged