Skip to content

Widen except clauses and align MockHTTPResponse for fixture swap#22864

Open
mwdd146980 wants to merge 25 commits intomwdd146980/httpx-migration-basefrom
mwdd146980/step3a-widen-om-exceptions
Open

Widen except clauses and align MockHTTPResponse for fixture swap#22864
mwdd146980 wants to merge 25 commits intomwdd146980/httpx-migration-basefrom
mwdd146980/step3a-widen-om-exceptions

Conversation

@mwdd146980
Copy link
Copy Markdown
Contributor

@mwdd146980 mwdd146980 commented Mar 10, 2026 •
edited
Loading

Motivation

Before swapping the mock_http_response fixture from MockResponse (requests-based) to MockHTTPResponse (library-agnostic), two things need to be true: MockHTTPResponse must match MockResponse behavior for properties tests rely on, and production except clauses must catch the library-agnostic exceptions raised by MockHTTPResponse.raise_for_status().

Approach

Aligned MockHTTPResponse with MockResponse behavior, then widened the 2 production except clauses exercised by error-path tests: OM V2 base.py and traefik_mesh _get_json(). Added ok/reason to HTTPResponseProtocol to match production usage. Individual integration except clauses will follow in a separate PR.

See plan for more details.

Review follow-ups

Addressed agint-review feedback:

  • Added test for dedent with indented multi-line content
  • Added transitional comment to MockHTTPResponse.ok
  • Added tests for library-agnostic exception handling in _get_json()
  • Removed unused url parameter from traefik_mesh.get_version()

See triage doc for full rationale on addressed/dismissed items.

Verification

ddev test -fs datadog_checks_base traefik_mesh
ddev --no-interactive test datadog_checks_base
ddev --no-interactive test traefik_mesh

@mwdd146980 mwdd146980 self-assigned this Mar 10, 2026
mwdd146980 and others added 22 commits March 20, 2026 13:56
@mwdd146980 @claude
Migrate gitlab_runner timeout test to http_client_session
0487a47 
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>
@mwdd146980
Reformat with ddev test --fmt
bbdb5c2
@mwdd146980 @claude
Migrate marathon config assertion test to http_client_session
70bf198 
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>
@mwdd146980 @claude
Migrate rabbitmq session-patch tests to http_client_session
a86d301 
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>
@mwdd146980 @claude
Migrate consul session-patch tests to http_client_session
ffc056c 
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>
@mwdd146980 @claude
Migrate torchserve session-patch test to http_client_session
aa74f49 
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>
@mwdd146980 @claude
Migrate etcd, ecs_fargate, mesos_master session-patch tests to http_c…
78bdf3b 
…lient_session

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980
Reformat with ddev test --fmt
d386336
@mwdd146980 @claude
Scrap http_client_session; migrate Step 2 tests to check.http.options…
ffebac2 
… 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>
@mwdd146980 @claude
Migrate airflow session-patch tests to mock_http
84bded7 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Migrate druid session-patch tests to mock_http
e522ca0 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Migrate mesos_slave session-patch tests to mock_http
66c6df3 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Migrate nginx session-patch tests to check.http.options and mock_http
7bfaf2b 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Migrate php_fpm session-patch tests to check.http.options and mock_http
05a25f1 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Add options to HTTPClientProtocol; initialize in mock_http fixture
5c31b82 
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>
@mwdd146980 @claude
Remove mock_http from http_testing.py; use datadog_checks_dev plugin …
13af852 
…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>
@mwdd146980 @claude
Fix mock_http fixture: initialize client.options as MagicMock
9df0547 
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>
@mwdd146980 @claude
Fix mock_http fixture: use real dict for client.options
44b71bc 
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>
@mwdd146980 @claude
Flatten test classes to top-level functions in test_headers.py and te…
a3ba255 
…st_http_testing.py

Match the dominant repo convention of top-level test functions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Make get_header/set_header case-insensitive
4392635 
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>
@mwdd146980 @claude
Align MockHTTPResponse + widen production except clauses (PR 3a)
ad32264 
MockHTTPResponse alignment (http_testing.py):
- Add textwrap.dedent() to string content normalization
- Add .ok and .reason properties

Production except clause widening (only clauses exercised by
mock_http_response error tests):
- openmetrics/v2/base.py: add HTTPRequestError, HTTPStatusError
- traefik_mesh/check.py: add HTTPStatusError, HTTPConnectionError

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mwdd146980 @claude
Address PR 3a review: protocol gaps, transitional comment, timeout ha…
731cea9 
…ndling

Add ok/reason properties to HTTPResponseProtocol to match production usage,
widen traefik_mesh timeout handler with HTTPTimeoutError, add transitional
comment to OM V2 except clause, and add tests for ok/reason on MockHTTPResponse.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 20, 2026
Draft
Draft
Draft
Merged
mwdd146980

This comment was marked as resolved.

Sign in to view

@mwdd146980 @claude
Address agint-review follow-ups: dedent test, exception tests, get_ve…
fd558c5 
…rsion fix

- Add test exercising dedent with indented multi-line content (item 2)
- Add transitional comment to MockHTTPResponse.ok property (item 3)
- Add tests for library-agnostic exception handling in _get_json (item 7)
- Remove unused url parameter from traefik_mesh get_version (item 9)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mwdd146980 mwdd146980 mentioned this pull request Mar 25, 2026
Draft
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lavigne958 lavigne958 lavigne958 approved these changes

Assignees

@mwdd146980 mwdd146980

Labels

agent/review-requested base_package ecosystems/review-requested integration/dcgm  integration/traefik_mesh product/review-requested team/agent-integrations team/container-integrations

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mwdd146980 @lavigne958