Migrate config assertion tests away from requests#22722
Merged
mwdd146980 merged 33 commits intomwdd146980/httpx-migration-basefrom Mar 19, 2026
Merged
Migrate config assertion tests away from requests#22722mwdd146980 merged 33 commits intomwdd146980/httpx-migration-basefrom
mwdd146980 merged 33 commits intomwdd146980/httpx-migration-basefrom
Conversation
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3 tasks
mwdd146980
changed the title
mwdd146980
changed the title
mwdd146980
force-pushed
the
mwdd146980/phase1b-step2-config-assertions
branch
from
ec60183 to
67059e7
Compare
This was referenced Feb 25, 2026
mwdd146980
changed the base branch from
mwdd146980/phase1b-test-decoupling
to
graphite-base/22722
mwdd146980
force-pushed
the
mwdd146980/phase1b-step2-config-assertions
branch
from
590936f to
fc636f0
Compare
mwdd146980
force-pushed
the
graphite-base/22722
branch
from
05fc099 to
f4df2b4
Compare
mwdd146980
changed the base branch from
graphite-base/22722
to
mwdd146980/phase1b-test-decoupling
Replace mock.patch('...requests.Session') + assert_called_with pattern
with the new http_client_session fixture. The full _request() option-merging
flow now runs, and the assertion verifies the timeout kwarg forwarded to
session.get().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace mock.patch('...requests.Session') + assert_called_with pattern
with the new http_client_session fixture. Also replace third-party mock
import with unittest.mock throughout the file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace mock.patch('...requests.Session') with the http_client_session
fixture in test__get_data (error-handling) and test_config (config
assertion).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace mock.patch('...requests.Session') patterns with the
http_client_session fixture in test_consul_request (error-handling)
and test_config (config assertion).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend _CanonicalMock with call_count, call_args, and call_args_list properties to support tests that introspect individual calls beyond assert_called_with. Then migrate test_get_models to use the fixture instead of patching requests.Session directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lient_session Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… and mock_http
- Remove http_client_session fixture, _CanonicalMock, and supporting code from
http_testing.py; update __all__ to ['MockHTTPResponse', 'mock_http']
- Config tests (couch, gitlab_runner, marathon, rabbitmq, consul, etcd,
ecs_fargate): drop http_client_session + check.check() + URL assertion;
assert check.http.options[key] == value directly — no HTTP call needed
- Functional tests (rabbitmq test__get_data, consul test_consul_request,
mesos_master test_can_connect_service_check, torchserve test_get_models):
replace http_client_session with mock_http
- mesos_master: set mock_http.options = {'verify': True} before check
instantiation since MesosMaster.__init__ reads self.http.options['verify']
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
create_autospec does not expose Protocol class-variable annotations at
runtime, so mock_http now explicitly sets client.options = MagicMock(spec=dict).
This provides a proper dict-like mock for any check that reads http.options
in __init__ (mesos_slave, mesos_master), removing the per-test workaround
of assigning mock_http.options = {'verify': True} before instantiation.
With options always present on the mock, the hasattr guard in
base_scraper.py is also removed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… etcd config test - Move timedelta and HTTPStatusError to top-level imports in http_testing.py - Complete HTTPResponseProtocol: add status_code, content, text, headers, json(), raise_for_status(), close() — previously the protocol only declared streaming methods and had no practical type safety - Remove inaccurate comment from test_authtoken.py (claimed MockHTTPResponse was a "drop-in replacement" and "httpx compatible") - Move etcd test_config out of test_integration.py into a new test_unit.py so it no longer requires a docker environment to run - Add mock_http fixture test (verifies PropertyMock patch reaches check.http) - Add MockHTTPResponse tests: iter_content chunking, iter_lines decode_unicode and custom delimiter, file_path constructor, raise_for_status 2xx no-op Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fixture mock_http was moved to datadog_checks_dev/plugin/pytest.py in PR #22710 and is auto-available to all tests via the plugin. Remove the duplicate definition from http_testing.py and drop the now-unnecessary noqa: F401 conftest imports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Protocol annotation-only attributes (options: dict[str, Any]) are not included in dir() and thus not auto-mocked by create_autospec. Checks like mesos_master that access self.http.options in __init__ raised AttributeError when using the mock_http fixture. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add get_header(name, default) and set_header(name, value) methods to RequestsWrapper and HTTPClientProtocol, replacing direct access to http.options['headers'][key] - Add TestGetHeader and TestSetHeader unit tests in test_headers.py - Migrate consul, php_fpm config tests to use get_header() - Migrate openmetrics tests to use get_header() - Migrate vault production code to use set_header() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MagicMock does not preserve dict item assignment, so any check that writes to self.http.options in __init__ would silently discard the mutation when tested with mock_http. Use a real dict with the standard RequestsWrapper keys so item assignment and retrieval work correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Vault legacy _set_header now delegates to http_wrapper.set_header() instead of direct dict mutation - mock_http fixture get_header/set_header now have side_effects that read/write client.options['headers'], matching real RequestsWrapper Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove trivial tests that duplicate stdlib behavior or are implicitly covered by other tests. Consolidate where possible (7→3 header tests, 14→8 http_testing tests). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…st_http_testing.py Match the dominant repo convention of top-level test functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both requests and httpx treat headers case-insensitively, so the API should too. Case folding happens inside the methods; options['headers'] remains a plain dict. set_header preserves the original key casing when overwriting an existing entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address steveny91's nit: the `assert isinstance(e, RabbitMQException)` lines inside `pytest.raises` blocks are dead code — execution leaves the with block when the exception is raised, so the assert never runs. Co-Authored-By: Claude Opus 4.6 (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 join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
What does this PR do?
Test Decoupling Step 2 (plan). Stacked on #22676.
Motivation
Many integration tests patch
requests.Sessionand useassert_called_withjust to verify config propagation. This couples tests to the HTTP transport library and requires running the full check with a mock response even though the values under test are available oncheck.http.optionsat construction time.Approach
check.http.optionsdirectly — no HTTP call, no session mock, nomock.ANYboilerplate. The gap betweenoptionsand actual session kwargs is already covered bytest_api.pyin the base package.mock_httpfixture from Step 1 instead of patchingrequests.Session.get_header()/set_header()toRequestsWrapperandHTTPClientProtocolso tests and production code (vault) don't reach intooptions['headers']directly.Verification
All 19 affected integrations pass their test suites. Detailed design notes: Confluence.
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged