diff --git a/.github/agents/axis-review-verify.agent.md b/.github/agents/axis-review-verify.agent.md index 836636e6..d2293d38 100644 --- a/.github/agents/axis-review-verify.agent.md +++ b/.github/agents/axis-review-verify.agent.md @@ -1,3 +1,25 @@ +# Axis Review Verify + +**Purpose:** +- Automated review and verification agent for Axis code changes. +- Runs all quality gates (tests, lint, type checks) and reports findings. +- Ensures PRs meet repository standards before merge. + +**Workflow:** +1. Run all checks: `uv run ruff check .`, `uv run ruff format --check .`, `uv run mypy axis`, `uv run pytest`. +2. Summarize failures and actionable findings. +3. If all checks pass, confirm PR is ready for review/merge. + +**Special Handling:** +- If coverage drops below 95%, block merge and report affected files. +- If pre-existing failures are unrelated to the PR, note them separately. +- If commit hooks modify files, re-stage and re-run checks. + +**Scope:** +- Use only for PR verification, not for architectural or design review. +- For code review, use the `Axis Review` agent. + +**See also:** `.github/copilot-instructions.md`, `CONTRIBUTING.md`. --- description: "Use when reviewing Axis changes and you want findings plus command-backed validation. Keywords: review with tests, verify PR, run targeted checks, regression verification, lint/type/test confirmation." name: "Axis Review Verify" diff --git a/.github/agents/axis-review.agent.md b/.github/agents/axis-review.agent.md index e23b021a..12478d74 100644 --- a/.github/agents/axis-review.agent.md +++ b/.github/agents/axis-review.agent.md @@ -4,7 +4,33 @@ name: "Axis Review" tools: [read, search] user-invocable: true --- -You are a specialized reviewer for the Axis Python repository. Your job is to find behavioral regressions, correctness risks, and missing test coverage before changes are merged. +## Axis Review + +**Purpose:** +- Automated code review agent for Axis repository. +- Reviews PRs for correctness, safety, and adherence to project conventions. +- Identifies regressions, missing tests, and risky changes. + +**Workflow:** +1. Review code changes for: + - Handler boundary discipline (models/interfaces separation) + - Enum normalization and `_missing_` fallbacks + - XML/event parsing safety (namespace-aware, root-shape guards) + - Test coverage and fixture usage + - Minimal, targeted changes +2. Summarize findings and actionable suggestions. +3. If risky or non-conventional changes are found, block merge and explain why. + +**Special Handling:** +- If tests, typing, or linting fail for unrelated reasons, note but do not block merge. +- If coverage drops below 95%, block merge and report affected files. +- If commit hooks modify files, re-stage and re-run checks. + +**Scope:** +- Use for code review and regression analysis. +- For full PR verification, use the `Axis Review Verify` agent. + +**See also:** `.github/copilot-instructions.md`, `CONTRIBUTING.md`. ## Scope - Review code changes and nearby context for defects and unintended behavior changes. diff --git a/.github/aiohttp-performance-matrix.md b/.github/aiohttp-performance-matrix.md deleted file mode 100644 index d5646c78..00000000 --- a/.github/aiohttp-performance-matrix.md +++ /dev/null @@ -1,38 +0,0 @@ -# Aiohttp Performance Evaluation Matrix - -## Context - -- Date: 2026-04-29 -- Branch: feature/aiohttp-test-migration -- Runtime: Python 3.14.2 via `uv run` -- Host: local development machine (macOS) - -## Method - -Each suite was executed 3 times with `pytest --no-cov` to reduce coverage overhead and -capture relative runtime consistency. - -Commands: - -1. `uv run pytest --no-cov tests/test_http_client_compat.py` -2. `uv run pytest --no-cov tests/test_vapix.py tests/test_ptz.py tests/test_view_areas.py tests/test_light_control.py` -3. `uv run pytest --no-cov tests/test_vapix.py tests/test_ptz.py tests/test_view_areas.py tests/test_light_control.py tests/test_mqtt.py tests/test_pwdgrp_cgi.py tests/test_stream_profiles.py tests/test_user_groups.py tests/test_event_instances.py tests/test_port_management.py tests/test_pir_sensor_configuration.py` - -## Results - -| Matrix Group | Passed Tests | Run 1 (s) | Run 2 (s) | Run 3 (s) | Avg (s) | Min (s) | Max (s) | -|---|---:|---:|---:|---:|---:|---:|---:| -| compat | 10 | 0.03 | 0.03 | 0.02 | 0.03 | 0.02 | 0.03 | -| handlers | 113 | 0.42 | 0.47 | 0.46 | 0.45 | 0.42 | 0.47 | -| phase6plus | 154 | 0.58 | 0.54 | 0.55 | 0.56 | 0.54 | 0.58 | - -## Interpretation - -1. Aiohttp-focused integration suites remain stable and low-latency on repeated runs. -2. Runtime spread is narrow for all groups, indicating no obvious regression drift. -3. The broader migrated suite (`phase6plus`) stays under one second across all samples. - -## Exit Decision - -Phase 9 acceptance is met: the aiohttp performance matrix has been executed and -recorded with repeatable timing data for representative migrated suites. \ No newline at end of file diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 84961b61..cd5be6d9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -40,6 +40,54 @@ ## Git Workflow +- Never create commits on the `master` branch. +- Never push commits directly to the `master` branch. +- Before any commit or push, check the current branch and confirm it is not `master`. +- If work is currently on `master`, create or switch to a feature branch before committing. +- If asked to commit or push from `master`, explain that the change must go through a feature branch and pull request. +- For any requested git operation, verify branch state first and summarize what will happen before committing or pushing. +# Project Guidelines + +## Build And Test + +- Use the project environment managed by `uv` (see `README.md`). +- Preferred bootstrap: `./setup.sh`. +- Python requirement is `>=3.14.0` (see `pyproject.toml`). +- Standard full checks: + - `uv run ruff check .` + - `uv run ruff format --check .` + - `uv run mypy axis` + - `uv run pytest` +- After code changes, run targeted tests for touched files first; run broader validation when shared behavior is affected. + +## Architecture + +- Keep boundaries clear: + - `axis/interfaces/` contains API handlers and transport-facing logic. + - `axis/models/` contains request/response models, enums, and parsing helpers. + - `axis/device.py` and `axis/interfaces/vapix.py` orchestrate device and handler lifecycle. +- Follow the phase-based handler initialization model documented in `README.md` (`API_DISCOVERY`, `PARAM_CGI_FALLBACK`, `APPLICATION`). +- Prefer boundary normalization for incoming values (for example, enum coercion and defaults in model constructors/post-init). + +## Conventions + +- Prefer minimal, targeted changes that preserve existing behavior unless the task explicitly requires a behavior change. +- Do not modify unrelated code, formatting, or tests. +- Never revert user changes unless explicitly asked. +- Before changing patterns or APIs, inspect nearby code and follow existing local style. +- Prefer root-cause fixes over workarounds. +- For enums and external inputs, preserve existing defensive normalization patterns (for example `_missing_` fallbacks and constructor normalization). +- For event/XML handling, preserve namespace-aware parsing and root-shape guards instead of assuming a fixed payload shape. + +## Testing Conventions + +- Add or update focused tests in the nearest relevant `tests/` module when behavior changes. +- Reuse existing async fixtures and HTTP mocking patterns from `tests/conftest.py`. +- If tests, typing, or linting fail for unrelated pre-existing reasons, report that clearly instead of fixing unrelated code. +- Expect commit hooks to run Ruff, Ruff format, and mypy; if hooks modify files, re-stage and re-run checks. + +## Git Workflow + - Never create commits on the `master` branch. - Never push commits directly to the `master` branch. - Before any commit or push, check the current branch and confirm it is not `master`. diff --git a/.github/final-architecture-signoff.md b/.github/final-architecture-signoff.md deleted file mode 100644 index 60ebfd34..00000000 --- a/.github/final-architecture-signoff.md +++ /dev/null @@ -1,78 +0,0 @@ -# Final Architecture Sign-Off And Rollback Gates - -## Decision Summary - -The aiohttp-first migration architecture is approved for completion based on: - -1. Test-layer migration from respx/httpx coupling to aiohttp_server parity coverage. -2. Consolidated shared shim utilities for migrated suites. -3. Runtime httpx-removal implementation plan documented and scoped. -4. Performance matrix confirming stable runtime behavior on representative suites. - -## Signed-Off Architecture State - -1. Initialization model remains phase-driven: -- API_DISCOVERY -- PARAM_CGI_FALLBACK -- APPLICATION - -2. Handler registration and grouped initialization behavior are preserved. - -3. Request/response behavior parity is maintained across migrated tests: -- status-code error translation, -- auth fallback behavior, -- namespace-aware/event parsing expectations. - -4. Planned runtime simplification is constrained and reversible: -- move to aiohttp-only runtime sessions, -- preserve existing request error semantics, -- preserve auth retry behavior for AUTO mode. - -## Release Gates - -All gates must be green before rollout: - -1. Linting: `uv run ruff check .` -2. Formatting: `uv run ruff format --check .` -3. Typing: `uv run mypy axis` -4. Tests: `uv run pytest` -5. Migration docs current: -- `.github/httpx-removal-implementation-plan.md` -- `.github/aiohttp-performance-matrix.md` - -## Rollback Gates And Triggers - -Rollback to pre-change baseline is required if any trigger occurs after runtime -httpx removal implementation begins: - -1. Functional trigger: -- auth negotiation failures or increased 401/403 responses in supported devices. - -2. Reliability trigger: -- sustained request timeout/connection-error regression outside historical bounds. - -3. Compatibility trigger: -- regressions in API discovery, param.cgi fallback, or application handler - initialization ordering. - -4. Quality trigger: -- failure to satisfy lint/type/test release gates. - -## Rollback Procedure - -1. Revert runtime-removal commits from the feature branch. -2. Restore last green commit set with full test migration and green CI. -3. Re-run full validation matrix. -4. Publish incident note in PR with root-cause and adjusted rollout plan. - -## Operational Notes - -1. Keep `uv.lock` updates isolated from migration intent unless dependency inputs - changed intentionally. -2. Preserve incremental commit discipline for any follow-up runtime changes. -3. Maintain PR checklist and commit ledger updates per increment. - -## Phase 10 Exit Criteria - -Phase 10 is complete when this sign-off is present in-repo and PR tracking marks -the final architecture sign-off and rollback gates as complete. \ No newline at end of file diff --git a/.github/httpx-removal-implementation-plan.md b/.github/httpx-removal-implementation-plan.md deleted file mode 100644 index a68ae994..00000000 --- a/.github/httpx-removal-implementation-plan.md +++ /dev/null @@ -1,108 +0,0 @@ -# Runtime HTTPX Removal Implementation Plan - -## Scope - -This plan defines how runtime support for `httpx` will be removed while preserving -existing behavior for Axis request handling, auth fallback, and error semantics. - -The test migration has already moved request/response test behavior to -`aiohttp_server`-backed flows. Remaining Phase 8 work is runtime-only. - -## Current Runtime Coupling (Inventory) - -1. `axis/interfaces/vapix.py` -- Imports `httpx` and supports two request engines (`httpx` and `aiohttp`). -- Constructs `httpx.BasicAuth` and `httpx.DigestAuth` for non-`aiohttp` sessions. -- Routes requests through `_perform_httpx_request` when session is not `aiohttp`. -- Maps `httpx.TimeoutException`, `httpx.TransportError`, and - `httpx.RequestError` to `RequestError`. -- Uses `httpx.DigestAuth` type checks for AUTO fallback behavior. - -2. `axis/models/configuration.py` -- `Configuration.session` is typed as `AsyncClient | ClientSession`. -- Type-checking import references `httpx.AsyncClient`. - -3. `pyproject.toml` -- Runtime dependency list includes `httpx>=0.26`. -- Optional pinned requirements include `httpx==0.28.1`. - -4. Tests -- `tests/test_configuration.py` still validates `httpx.AsyncClient` acceptance in - `Configuration`. -- `tests/test_vapix.py` and `tests/respx_shim.py` reference `httpx` exceptions to - assert request-error translation behavior. -- `tests/test_http_client_compat.py` already validates `aiohttp` runtime behavior. - -## Target Runtime Contract - -1. Runtime session type is `aiohttp.ClientSession` only. -2. Runtime auth types are `aiohttp.BasicAuth` or digest handled by - `AiohttpDigestAuth` / `DigestAuthMiddleware` when available. -3. Request execution path is single-engine (`aiohttp`) only. -4. Public behavior remains unchanged: -- same `RequestError` mapping for timeout/connection/general request failures, -- same status-code-to-Axis-error mapping, -- same AUTO auth fallback semantics. - -## Ordered Execution Plan - -1. Narrow configuration typing to `aiohttp.ClientSession` -- Update `axis/models/configuration.py` to remove `httpx.AsyncClient` references - and define `HTTPSession = ClientSession`. -- Update tests that currently expect mixed-session acceptance. - -2. Remove dual-engine request logic from `Vapix` -- Delete `_perform_httpx_request`, `_httpx_session`, `_httpx_auth`, and - `_client_name` branching. -- Remove `httpx.BasicAuth`/`httpx.DigestAuth` construction paths. -- Keep `_perform_aiohttp_request` as the single request engine. - -3. Preserve exception mapping semantics without `httpx` -- Replace explicit `httpx` exception branches with `aiohttp` and generic timeout - handling while preserving current `RequestError` messages: - - timeout -> `RequestError("Timeout")` - - connection issues -> `RequestError("Connection error: ...")` - - other client errors -> `RequestError("Unknown error: ...")` - -4. Adjust AUTO auth fallback checks -- Replace `httpx.DigestAuth`-specific checks in `_should_retry_with_basic` with - client-agnostic state checks that still enforce one retry. - -5. Update dependency metadata -- Remove `httpx` from runtime dependencies in `pyproject.toml`. -- Remove pinned `httpx` entry from `project.optional-dependencies.requirements`. - -6. Update tests to reflect aiohttp-only runtime -- Replace or remove tests that assert `httpx.AsyncClient` compatibility in - `tests/test_configuration.py`. -- Keep and extend `tests/test_http_client_compat.py` for auth and request parity. -- Update `tests/test_vapix.py` and `tests/respx_shim.py` to avoid direct `httpx` - exception dependencies while preserving equivalent behavior assertions. - -7. Validation gates -- `uv run ruff check axis tests` -- `uv run ruff format --check axis tests` -- `uv run mypy axis` -- targeted pytest for touched suites first -- `uv run pytest` - -## Risks And Mitigations - -1. Risk: subtle auth regression in AUTO mode. -- Mitigation: retain and expand fallback tests around `WWW-Authenticate` handling. - -2. Risk: changed exception text breaks downstream consumers/tests. -- Mitigation: preserve existing `RequestError` message strings verbatim. - -3. Risk: digest middleware availability differs by aiohttp version. -- Mitigation: keep current middleware availability guard and digest helper fallback. - -## Exit Criteria - -Phase 8 is complete when all are true: - -1. Runtime package no longer depends on `httpx`. -2. Production code under `axis/` has no `import httpx` references. -3. `Configuration` and `Vapix` are aiohttp-only at runtime. -4. Existing request/auth/error behavior is preserved by tests. -5. Full lint/type/test matrix passes. \ No newline at end of file diff --git a/.github/mock-consolidation-architecture.md b/.github/mock-consolidation-architecture.md deleted file mode 100644 index f7a2962f..00000000 --- a/.github/mock-consolidation-architecture.md +++ /dev/null @@ -1,250 +0,0 @@ -# Aiohttp Mock Server Consolidation Architecture - -## Problem Statement - -**Duplication Scope Audit:** -- **53 instances** of `app = web.Application()` across test suite -- **174 aiohttp_server usages** with repetitive route setup -- **~300+ lines** of boilerplate handler/router/server creation code - -**Anti-Patterns Eliminated:** -```python -# BEFORE: Repeated across 53+ test functions -app = web.Application() -app.router.add_post("/path", handler) -server = await aiohttp_server(app) -device.config.port = server.port -``` - -## Solution: Consolidated Mock Factory Fixture - -**Location:** `tests/conftest.py::aiohttp_mock_server` - -### Architecture Decision - -**Pattern:** Fixture-based dependency injection with fluent API -**Benefit:** Single source of truth for mock server setup -**Impact:** ~300 lines of test boilerplate eliminated - -### Fixture Capabilities - -#### 1. Single Route (Most Common) -```python -# BEFORE (manual handler + app setup) -requests = [] -async def handle_request(request): - requests.append({"method": ..., "path": ...}) - return web.json_response(data) - -app = web.Application() -app.router.add_post("/api/endpoint", handle_request) -server = await aiohttp_server(app) -device.config.port = server.port - -# AFTER (consolidated fixture) -server, requests = await aiohttp_mock_server( - "/api/endpoint", - response={"data": []}, - device=device, -) -``` - -#### 2. Multiple Routes -```python -# BEFORE -app = web.Application() -app.router.add_post("/path1", handler1) -app.router.add_post("/path2", handler2) -server = await aiohttp_server(app) - -# AFTER -server, requests = await aiohttp_mock_server({ - "/path1": handler1, - "/path2": handler2, -}) -``` - -#### 3. Response-Only Routes (No Custom Logic) -```python -# BEFORE -async def handle_status(request): - requests.append({"method": ..., "path": ...}) - return web.Response(text="ok") - -# AFTER (auto-handler) -server, requests = await aiohttp_mock_server({ - "/status": { - "method": "GET", - "response": "ok", - "status": 200, - } -}) -``` - -#### 4. Mixed JSON/Text/Binary Responses -```python -server, requests = await aiohttp_mock_server({ - "/api/data": { - "response": {"key": "value"}, # auto JSON - }, - "/api/text": { - "response": "plain text", # auto text - }, - "/api/binary": { - "response": b"\x00\x01", # auto binary - "status": 201, - } -}) -``` - -### Request Capture Pattern - -All routes automatically capture requests when `capture_requests=True` (default): - -```python -server, requests = await aiohttp_mock_server( - "/api/method", - response={"result": "ok"}, -) - -# requests is a list of dicts: -# [{"method": "POST", "path": "/api/method", "query": ""}] -``` - -### Device Config Binding - -Auto-bind server port to device config: - -```python -server, requests = await aiohttp_mock_server( - "/api/endpoint", - handler=my_handler, - device=axis_device, # auto-sets device.config.port -) -# No manual: device.config.port = server.port -``` - -## Migration Path - -### Phase 1: Targeted Refactoring (By Module) -1. `tests/test_basic_device_info.py` (2 instances) -2. `tests/test_mqtt.py` (1 instance via helper) -3. `tests/test_port_management.py` (5 instances) -4. ... continue module-by-module - -### Phase 2: Validation -- Run full test suite: `uv run pytest` -- Verify request capture parity -- Confirm device port binding - -### Phase 3: Follow-up Refactoring -- Eliminate remaining 40+ instances -- Update application/ and parameters/ suites - -## Example Refactorings - -### Test: test_basic_device_info.py::test_get_all_properties - -**BEFORE:** -```python -async def test_get_all_properties(aiohttp_server): - requests = [] - - async def handle_basic_device_info(request: web.Request) -> web.Response: - payload = await request.json() - requests.append({"method": request.method, "path": request.path, "payload": payload}) - return web.json_response(GET_ALL_PROPERTIES_RESPONSE) - - app = web.Application() - app.router.add_post("/axis-cgi/basicdeviceinfo.cgi", handle_basic_device_info) - server = await aiohttp_server(app) - - session = aiohttp.ClientSession() - axis_device = AxisDevice(Configuration(session, HOST, port=server.port, ...)) - # test continues... -``` - -**AFTER:** -```python -async def test_get_all_properties(aiohttp_mock_server, aiohttp_session): - axis_device = AxisDevice(Configuration(aiohttp_session, HOST, ...)) - - server, requests = await aiohttp_mock_server( - "/axis-cgi/basicdeviceinfo.cgi", - response=GET_ALL_PROPERTIES_RESPONSE, - device=axis_device, - ) - - # test continues (same assertions on requests) -``` - -**Lines saved:** 11 → 7 (36% reduction per test) - -### Test: test_applications.py::test_update_multiple_applications - -**BEFORE:** -```python -async def test_update_multiple_applications(aiohttp_server, applications): - async def handle_request(_: web.Request) -> web.Response: - return web.Response( - text=LIST_APPLICATIONS_RESPONSE, - headers={"Content-Type": "text/xml"}, - ) - - app = web.Application() - app.router.add_post("/axis-cgi/applications/list.cgi", handle_request) - server = await aiohttp_server(app) - applications.vapix.device.config.port = server.port - - await applications.update() - # assertions... -``` - -**AFTER:** -```python -async def test_update_multiple_applications(aiohttp_mock_server, applications): - server, _ = await aiohttp_mock_server( - "/axis-cgi/applications/list.cgi", - response=LIST_APPLICATIONS_RESPONSE, - headers={"Content-Type": "text/xml"}, - device=applications.vapix, - ) - - await applications.update() - # assertions... -``` - -**Lines saved:** 12 → 9 (25% reduction) - -## Benefits Summary - -| Aspect | Impact | -|--------|--------| -| **Duplication Reduction** | 53 instances eliminated (~300 LOC) | -| **Request Capture** | Built-in for all routes | -| **Device Port Binding** | Automatic, no manual assignment | -| **Response Flexibility** | JSON/text/binary auto-detection | -| **Handler Composition** | Custom handlers still supported | -| **Test Readability** | Intent-focused, setup minimized | -| **Maintenance Burden** | Single fixture vs. scattered code | - -## Non-Breaking Compatibility - -Existing tests using manual `app = web.Application()` remain unaffected: -- Fixture is additive (does not change existing behavior) -- Existing tests pass without modification -- Gradual migration possible (test-by-test) -- Rollback is trivial (fixture removal has no side effects) - -## Rollback & Risk Mitigation - -**Rollback Trigger:** If fixture introduces false test results -**Rollback Procedure:** Remove `aiohttp_mock_server` fixture; tests revert to explicit setup -**Risk:** Very low (fixture is pure implementation detail) - -## Next Steps - -1. Validate fixture with trial refactoring of 2-3 tests -2. Run full test matrix to ensure parity -3. Document module-by-module refactoring roadmap -4. Gradually migrate remaining 50+ instances diff --git a/.github/mock-fixture-pilot-summary.md b/.github/mock-fixture-pilot-summary.md deleted file mode 100644 index 7c6fff4d..00000000 --- a/.github/mock-fixture-pilot-summary.md +++ /dev/null @@ -1,323 +0,0 @@ -# Mock Server Fixture Consolidation - Pilot Summary - -**Status:** ✅ PILOT COMPLETE & VALIDATED -**Date:** 2025 -**Scope:** tests/applications/test_applications.py (4 tests refactored) -**Branch:** feature/aiohttp-test-migration - ---- - -## Executive Summary - -Successfully piloted consolidated `aiohttp_mock_server` fixture on 4 tests in `tests/applications/test_applications.py`. All tests pass with parity to prior behavior. Refactoring eliminates **47 lines of repetitive boilerplate** (68% reduction) across the 4 test functions while improving readability and maintainability. - ---- - -## Pilot Results - -| Test File | Test Function | Before LOC | After LOC | Reduction | Feature | -|---|---|---|---|---|---| -| test_applications.py | test_update_no_application | 17 | 7 | -59% | Response specs | -| test_applications.py | test_update_single_application | 18 | 8 | -56% | Response specs | -| test_applications.py | test_update_multiple_applications | 22 | 10 | -55% | Response specs | -| test_applications.py | test_responses_with_with_limitations | 12 | 7 | -42% | Response specs | -| test_fence_guard.py | test_get_empty_configuration | 23 | 10 | -57% | Payload capture | -| test_fence_guard.py | test_get_configuration | 18 | 7 | -61% | Response specs | -| **TOTAL (6 tests)** | **-** | **110** | **49** | **-55%** | **Multi-feature** | - -### Cumulative Impact -- **Total refactored tests:** 6 (4 + 2) -- **Average per-test reduction:** -10.2 LOC (-55%) -- **Total boilerplate eliminated:** 61 LOC -- **Fixture maturity:** Extended with payload capture capability - ---- - -## Before: Original Implementation - -```python -# tests/applications/test_applications.py (BEFORE) - -async def test_update_no_application(aiohttp_server, applications): - """Test update applicatios call.""" - requests = [] - - async def handle_request(request): - requests.append({"method": request.method, "path": request.path}) - return web.Response( - text=LIST_APPLICATION_EMPTY_RESPONSE, - headers={"Content-Type": "text/xml"}, - ) - - app = web.Application() # <-- BOILERPLATE #1 - app.router.add_post("/axis-cgi/applications/list.cgi", handle_request) # <-- BOILERPLATE #2 - server = await aiohttp_server(app) - applications.vapix.device.config.port = server.port # <-- BOILERPLATE #3 - - await applications.update() - - assert requests - assert requests[-1]["method"] == "POST" - assert requests[-1]["path"] == "/axis-cgi/applications/list.cgi" - assert len(applications.values()) == 0 - # Total: 17 lines (9 boilerplate + 8 test logic) -``` - -**Issues in original:** -- Manual request capture list (prone to inconsistency) -- Inline handler definition (repeated pattern) -- Manual app/router setup (web.Application boilerplate) -- Manual device port binding (easy to forget or mistype) -- 9 lines of setup before first assertion - ---- - -## After: Consolidated Fixture - -```python -# tests/applications/test_applications.py (AFTER - same test refactored) - -async def test_update_no_application(aiohttp_mock_server, applications): - """Test update applicatios call.""" - server, requests = await aiohttp_mock_server( - "/axis-cgi/applications/list.cgi", - response=LIST_APPLICATION_EMPTY_RESPONSE, - headers={"Content-Type": "text/xml"}, - device=applications, - ) - - await applications.update() - - assert requests - assert requests[-1]["method"] == "POST" - assert requests[-1]["path"] == "/axis-cgi/applications/list.cgi" - assert len(applications.values()) == 0 - # Total: 7 lines (1 fixture call + 6 test logic) -``` - -**Improvements:** -- Single fixture call replaces 9 lines of setup -- Request capture automatic and consistent -- Handler auto-generated from response spec -- Device binding automatic and bulletproof -- Response spec is declarative (easier to read) - ---- - -## Fixture Capability Demonstration - -The new fixture supports four usage patterns (all validated): - -### Pattern 1: Single Route with Response Spec (Pilot used this) -```python -server, requests = await aiohttp_mock_server( - "/api/endpoint", - response={"key": "value"}, # JSON auto-detected - device=handler_or_device, # Auto-binds port -) -``` - -### Pattern 2: Single Route with Custom Handler -```python -async def custom_handler(request): - return web.json_response({"custom": True}) - -server, requests = await aiohttp_mock_server( - "/api/endpoint", - handler=custom_handler, - device=handler_or_device, -) -``` - -### Pattern 3: Multiple Routes with Mixed Specs -```python -server, requests = await aiohttp_mock_server( - { - "/path1": {"method": "GET", "response": "text response"}, - "/path2": {"method": "POST", "response": {"json": "data"}}, - "/path3": custom_handler_func, # Mix specs and handlers - }, - device=device, -) -``` - -### Pattern 4: Multiple Routes with Request Inspection -```python -server, requests = await aiohttp_mock_server( - "/api/list", - response=XML_RESPONSE_DATA, - headers={"Content-Type": "text/xml"}, - device=applications, -) -# Access all captured requests: requests[0], requests[1], etc. -``` - -### Pattern 5: Payload Capture (NEW in Extended Fixture) -```python -server, requests = await aiohttp_mock_server( - "/api/control", - response={"status": "ok"}, - device=handler, - capture_payload=True, # Auto-captures JSON payloads -) -# Access request payloads: requests[0]["payload"], etc. -# Eliminates: requests = [], await request.json() boilerplate -``` - ---- - -## Code Reduction Details (4 Tests) - -### Test 1: `test_update_no_application` -- Before: 17 LOC -- After: 7 LOC -- Reduction: **10 LOC (59%)** - -### Test 2: `test_update_single_application` -- Before: 18 LOC -- After: 8 LOC -- Reduction: **10 LOC (56%)** - -### Test 3: `test_update_multiple_applications` -- Before: 22 LOC -- After: 10 LOC -- Reduction: **12 LOC (55%)** - -### Test 4: `test_responses_with_with_limitations` -- Before: 12 LOC -- After: 7 LOC -- Reduction: **5 LOC (42%)** - -**Average per-test reduction: 68% code elimination** - ---- - -## Validation Results - -### Test Execution (6 Tests) -```bash -$ uv run pytest tests/applications/test_applications.py tests/applications/test_fence_guard.py --no-cov -v -collected 6 items -test_applications.py::test_update_no_application PASSED -test_applications.py::test_update_single_application PASSED -test_applications.py::test_update_multiple_applications PASSED -test_applications.py::test_responses_with_with_limitations PASSED -test_fence_guard.py::test_get_empty_configuration PASSED -test_fence_guard.py::test_get_configuration PASSED - -====== 6 passed in 0.03s ====== -``` - -### Code Quality Checks -- ✅ Syntax validation: conftest.py passed `uv run python -m py_compile` -- ✅ Type checking: Fixture fully typed with Callable, dict, etc. -- ✅ Linting: Ruff checks pass (specific exception handling) -- ✅ Request capture: Automatic (no manual list management) -- ✅ Device binding: Polymorphic (supports AxisDevice, Vapix, ApiHandler) -- ✅ Payload capture: JSON/text auto-detection works, error handling graceful - ---- - -## Pilot Learnings - -### What Worked Well -1. **Response spec DSL** — Declarative, eliminates handler boilerplate -2. **Request capture** — Automatic, always consistent (can't forget to append) -3. **Payload capture** — Auto-reads JSON/text from requests, eliminates manual await request.json() -4. **Device binding polymorphism** — Works with handlers, Vapix, and AxisDevice -5. **Backward compatibility** — Existing tests unaffected; migration optional -6. **Clear attribute mapping** — Handlers → vapix.device.config; Vapix → device.config; AxisDevice → config - -### Design Decisions Validated -1. **Fixture returns (server, requests) tuple** — Ergonomic, matches xfail/aiohttp_server patterns -2. **Auto-handler via response spec** — Reduces cognitive load (no lambda boilerplate) -3. **Payload capture as opt-in parameter** — Flexible for tests that need request inspection -4. **Support for custom handlers alongside specs** — Works with incremental migration -5. **Specific exception handling** — Catches ValueError/RuntimeError, not blind Exception - -### Edge Cases Handled -- ✅ ApiHandler device binding (has vapix attribute) -- ✅ Vapix device binding (has device attribute) -- ✅ Direct AxisDevice binding (has config attribute) -- ✅ Mixed JSON/text/binary responses -- ✅ Multiple routes with mixed response types -- ✅ Request capture with custom handlers -- ✅ **NEW:** JSON payload capture (eliminates manual await request.json()) -- ✅ **NEW:** Graceful handling when payload reading fails - ---- - -## Migration Roadmap (Next Phases) - -### ✅ COMPLETED: Pilot Phase (6 tests) -- `tests/applications/test_applications.py` — 4 tests refactored -- `tests/applications/test_fence_guard.py` — 2 tests refactored -- **Actual savings: 61 LOC (-55%)** -- **Fixture enhancements: Payload capture added** - -### Phase 1: Core Handlers (High Duplication) -- `tests/test_basic_device_info.py` — 3 instances (~15 LOC savings) -- `tests/test_mqtt.py` — 5 instances (~25 LOC savings) -- `tests/test_port_management.py` — 6 instances (~30 LOC savings) -- **Estimated savings: 70 LOC** - -### Phase 2: Application Subdirectory (Remaining) -- `tests/applications/test_loitering_guard.py` — 2 instances -- `tests/applications/test_motion_guard.py` — 2 instances -- `tests/applications/test_object_analytics.py` — 3 instances -- `tests/applications/test_vmd4.py` — 2 instances -- **Estimated savings: 45 LOC** - -### Phase 3: API Discovery & Event Handlers -- `tests/test_api_discovery.py` — 4 instances -- `tests/test_event_instances.py` — 3 instances -- `tests/test_user_groups.py` — 2 instances -- **Estimated savings: 45 LOC** - -### Phase 4: Parameter Tests -- `tests/parameters/test_*.py` (15 test files) -- Bulk refactoring with single-fixture pattern -- **Estimated savings: 120 LOC** - -### Total Remaining (After Pilot): ~280 LOC savings across 40+ instances - ---- - -## Risk Assessment - -### Low Risk ✅ -- Fixture is pure helper (no breaking API changes) -- Request capture is opt-in (existing tests unaffected) -- All existing tests remain valid and pass -- Can migrate incrementally, test-by-test - -### Mitigations -1. **Gradual rollout** — Phase-by-phase migration with validation between phases -2. **No forced changes** — Existing tests work as-is; new tests use fixture -3. **Test parity** — All refactored tests pass with identical assertions -4. **Documentation** — Pilot summary + architecture doc + inline fixture docstring - ---- - -## Next Steps - -1. ✅ **Pilot Phase Complete** — 6 tests refactored, payload capture feature validated -2. ✅ **Phase 1 Ready** — Apply fixture to core handlers (test_basic_device_info.py, test_mqtt.py, test_port_management.py) -3. ✅ **Documentation Updated** — Fixture now documented with payload capture usage -4. ✅ **Incremental Validation** — Run full test suite after each phase -5. ✅ **Update Architecture** — Extend CONTRIBUTING.md with fixture usage guide -6. ✅ **Archive Extended Pilot** — Commit this summary with payload capture proof-of-concept - ---- - -## Artifact References - -- **Fixture Implementation:** [tests/conftest.py](../tests/conftest.py#L270-L320) -- **Pilot Tests:** [tests/applications/test_applications.py](../tests/applications/test_applications.py#L20-L200) -- **Fence Guard Tests:** [tests/applications/test_fence_guard.py](../tests/applications/test_fence_guard.py#L20-L75) -- **Architecture Doc:** [.github/mock-consolidation-architecture.md](./mock-consolidation-architecture.md) -- **Branch:** feature/aiohttp-test-migration (PR #776) - ---- - -**Conclusion:** Extended pilot validates that consolidated fixture with payload capture is production-ready. Phase 1-4 refactoring can proceed with confidence. Projected ROI is 280+ LOC reduction (after pilot) with 100% test pass rate maintained. diff --git a/axis/__main__.py b/axis/__main__.py index df7156fe..da045b4f 100644 --- a/axis/__main__.py +++ b/axis/__main__.py @@ -113,11 +113,7 @@ async def main( device.stream.stop() finally: - session = device.config.session - if not isinstance(session, aiohttp.ClientSession): - message = "Configured session is not an aiohttp ClientSession" - raise RuntimeError(message) - await close_session(session) + await close_session(device.config.session) device.stream.stop() diff --git a/axis/interfaces/aiohttp_digest.py b/axis/interfaces/aiohttp_digest.py index 396d8eae..a1e637ef 100644 --- a/axis/interfaces/aiohttp_digest.py +++ b/axis/interfaces/aiohttp_digest.py @@ -35,7 +35,7 @@ def should_use_library_digest(self, http_client: str, has_basic_auth: bool) -> b """Return if aiohttp requests should use library-managed digest auth. Args: - http_client: Name of HTTP client ("aiohttp" or "httpx"). + http_client: Name of HTTP client. has_basic_auth: Whether basic auth is configured. Returns: diff --git a/axis/interfaces/vapix.py b/axis/interfaces/vapix.py index 000a49b6..c30368df 100644 --- a/axis/interfaces/vapix.py +++ b/axis/interfaces/vapix.py @@ -7,7 +7,6 @@ from typing import TYPE_CHECKING, Any, cast import aiohttp -import httpx from ..errors import RequestError, raise_error from ..models.configuration import AuthScheme @@ -55,20 +54,14 @@ class Vapix: def __init__(self, device: AxisDevice) -> None: """Store local reference to device config.""" self.device = device - self._http_client = self._client_name() self._aiohttp_digest_middleware: Any | None = None self._aiohttp_digest_auth = AiohttpDigestAuth(device) - if self._http_client == "aiohttp": - if device.config.auth_scheme == AuthScheme.BASIC: - self.auth = self._aiohttp_basic_auth() - else: - self.auth = None - self._aiohttp_digest_middleware = self._aiohttp_digest_middleware_obj() - elif device.config.auth_scheme == AuthScheme.BASIC: - self.auth = httpx.BasicAuth(device.config.username, device.config.password) + if device.config.auth_scheme == AuthScheme.BASIC: + self.auth = self._aiohttp_basic_auth() else: - self.auth = httpx.DigestAuth(device.config.username, device.config.password) + self.auth = None + self._aiohttp_digest_middleware = self._aiohttp_digest_middleware_obj() # Grouped handlers are registered in handler-construction order. # Handlers with empty handler_groups are intentionally excluded. @@ -327,20 +320,6 @@ async def _request( params=params, ) - except httpx.TimeoutException as errt: - message = "Timeout" - raise RequestError(message) from errt - - except httpx.TransportError as errc: - LOGGER.debug("%s", errc) - message = f"Connection error: {errc}" - raise RequestError(message) from errc - - except httpx.RequestError as err: - LOGGER.debug("%s", err) - message = f"Unknown error: {err}" - raise RequestError(message) from err - except TimeoutError as errt: message = "Timeout" raise RequestError(message) from errt @@ -391,17 +370,8 @@ async def _perform_request( headers: dict[str, str] | None, params: dict[str, str] | None, ) -> tuple[int, dict[str, str], bytes]: - """Execute request and normalize responses from supported HTTP clients.""" - if self._http_client == "aiohttp": - return await self._perform_aiohttp_request( - method=method, - url=url, - content=content, - data=data, - headers=headers, - params=params, - ) - return await self._perform_httpx_request( + """Execute request and normalize responses.""" + return await self._perform_aiohttp_request( method=method, url=url, content=content, @@ -410,29 +380,6 @@ async def _perform_request( params=params, ) - async def _perform_httpx_request( - self, - method: str, - url: str, - content: bytes | None, - data: dict[str, str] | None, - headers: dict[str, str] | None, - params: dict[str, str] | None, - ) -> tuple[int, dict[str, str], bytes]: - """Execute request with a httpx session.""" - session = self._httpx_session() - response = await session.request( - method, - url, - content=content, - data=data, - headers=headers, - params=params, - auth=self._httpx_auth(), - timeout=TIME_OUT, - ) - return response.status_code, dict(response.headers), response.content - async def _perform_aiohttp_request( self, method: str, @@ -449,8 +396,7 @@ async def _perform_aiohttp_request( session = self._aiohttp_session() if ( - self._http_client == "aiohttp" - and not self._aiohttp_auth() + not self._aiohttp_auth() and self.device.config.auth_scheme != AuthScheme.BASIC ): return await self._aiohttp_digest_auth.perform_request( @@ -471,17 +417,9 @@ async def _perform_aiohttp_request( response_content = await response.read() return response.status, dict(response.headers), response_content - def _httpx_session(self) -> httpx.AsyncClient: - """Return session cast to a httpx client.""" - return cast("httpx.AsyncClient", self.device.config.session) - def _aiohttp_session(self) -> ClientSession: """Return session cast to an aiohttp client.""" - return cast("ClientSession", self.device.config.session) - - def _httpx_auth(self) -> httpx.Auth | None: - """Return auth cast for httpx requests.""" - return cast("httpx.Auth | None", self.auth) + return self.device.config.session def _aiohttp_auth(self) -> AiohttpBasicAuth | None: """Return auth cast for aiohttp requests.""" @@ -509,10 +447,8 @@ def _aiohttp_digest_middleware_obj(self) -> Any | None: ) def _basic_auth(self) -> object: - """Create basic auth object for configured HTTP client.""" - if self._http_client == "aiohttp": - return self._aiohttp_basic_auth() - return httpx.BasicAuth(self.device.config.username, self.device.config.password) + """Create basic auth object.""" + return self._aiohttp_basic_auth() def _aiohttp_basic_auth(self) -> object: """Create aiohttp basic auth object.""" @@ -520,12 +456,6 @@ def _aiohttp_basic_auth(self) -> object: self.device.config.username, self.device.config.password ) - def _client_name(self) -> str: - """Return normalized client name from configured session object.""" - if isinstance(self.device.config.session, aiohttp.ClientSession): - return "aiohttp" - return "httpx" - def _should_retry_with_basic( self, headers: dict[str, str], allow_auto_basic_retry: bool ) -> bool: @@ -536,9 +466,6 @@ def _should_retry_with_basic( if self.device.config.auth_scheme != AuthScheme.AUTO: return False - if self._http_client == "httpx" and not isinstance(self.auth, httpx.DigestAuth): - return False - expected_auth = "" for header_name, header_value in headers.items(): if header_name.lower() == "www-authenticate": diff --git a/axis/models/configuration.py b/axis/models/configuration.py index 5da87ec6..41b0a529 100644 --- a/axis/models/configuration.py +++ b/axis/models/configuration.py @@ -7,9 +7,8 @@ if TYPE_CHECKING: from aiohttp import ClientSession - from httpx import AsyncClient - type HTTPSession = AsyncClient | ClientSession + type HTTPSession = ClientSession LOGGER = logging.getLogger(__name__) diff --git a/pyproject.toml b/pyproject.toml index e4ed6c87..f5687bd5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,6 @@ requires-python = ">=3.14.0" dependencies = [ "aiohttp>=3.12", "faust-cchardet>=2.1.18", - "httpx>=0.26", "orjson>3.9", "packaging>23", "xmltodict>=0.13.0", @@ -31,7 +30,6 @@ dependencies = [ [project.optional-dependencies] requirements = [ "aiohttp==3.13.5", - "httpx==0.28.1", "orjson==3.11.8", "packaging==26.2", "xmltodict==1.0.4", diff --git a/uv.lock b/uv.lock index dd431751..44969ba8 100644 --- a/uv.lock +++ b/uv.lock @@ -78,18 +78,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/fb/76/641ae371508676492379f16e2fa48f4e2c11741bd63c48be4b12a6b09cba/aiosignal-1.4.0-py3-none-any.whl", hash = "sha256:053243f8b92b990551949e63930a839ff0cf0b0ebbe0597b0f3fb19e1a0fe82e", size = 7490, upload-time = "2025-07-03T22:54:42.156Z" }, ] -[[package]] -name = "anyio" -version = "4.13.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "idna" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/19/14/2c5dd9f512b66549ae92767a9c7b330ae88e1932ca57876909410251fe13/anyio-4.13.0.tar.gz", hash = "sha256:334b70e641fd2221c1505b3890c69882fe4a2df910cba14d97019b90b24439dc", size = 231622, upload-time = "2026-03-24T12:59:09.671Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/da/42/e921fccf5015463e32a3cf6ee7f980a6ed0f395ceeaa45060b61d86486c2/anyio-4.13.0-py3-none-any.whl", hash = "sha256:08b310f9e24a9594186fd75b4f73f4a4152069e3853f1ed8bfbf58369f4ad708", size = 114353, upload-time = "2026-03-24T12:59:08.246Z" }, -] - [[package]] name = "attrs" version = "26.1.0" @@ -106,7 +94,6 @@ source = { editable = "." } dependencies = [ { name = "aiohttp" }, { name = "faust-cchardet" }, - { name = "httpx" }, { name = "orjson" }, { name = "packaging" }, { name = "xmltodict" }, @@ -115,7 +102,6 @@ dependencies = [ [package.optional-dependencies] requirements = [ { name = "aiohttp" }, - { name = "httpx" }, { name = "orjson" }, { name = "packaging" }, { name = "xmltodict" }, @@ -138,8 +124,6 @@ requires-dist = [ { name = "aiohttp", specifier = ">=3.12" }, { name = "aiohttp", marker = "extra == 'requirements'", specifier = "==3.13.5" }, { name = "faust-cchardet", specifier = ">=2.1.18" }, - { name = "httpx", specifier = ">=0.26" }, - { name = "httpx", marker = "extra == 'requirements'", specifier = "==0.28.1" }, { name = "mypy", marker = "extra == 'requirements-test'", specifier = "==1.20.2" }, { name = "orjson", specifier = ">3.9" }, { name = "orjson", marker = "extra == 'requirements'", specifier = "==3.11.8" }, @@ -157,15 +141,6 @@ requires-dist = [ ] provides-extras = ["requirements", "requirements-test", "requirements-dev"] -[[package]] -name = "certifi" -version = "2026.2.25" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/af/2d/7bf41579a8986e348fa033a31cdd0e4121114f6bce2457e8876010b092dd/certifi-2026.2.25.tar.gz", hash = "sha256:e887ab5cee78ea814d3472169153c2d12cd43b14bd03329a39a9c6e2e80bfba7", size = 155029, upload-time = "2026-02-25T02:54:17.342Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/9a/3c/c17fb3ca2d9c3acff52e30b309f538586f9f5b9c9cf454f3845fc9af4881/certifi-2026.2.25-py3-none-any.whl", hash = "sha256:027692e4402ad994f1c42e52a4997a9763c646b73e4096e4d5d6db8af1d6f0fa", size = 153684, upload-time = "2026-02-25T02:54:15.766Z" }, -] - [[package]] name = "cfgv" version = "3.5.0" @@ -288,43 +263,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9a/9a/e35b4a917281c0b8419d4207f4334c8e8c5dbf4f3f5f9ada73958d937dcc/frozenlist-1.8.0-py3-none-any.whl", hash = "sha256:0c18a16eab41e82c295618a77502e17b195883241c563b00f0aa5106fc4eaa0d", size = 13409, upload-time = "2025-10-06T05:38:16.721Z" }, ] -[[package]] -name = "h11" -version = "0.16.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/01/ee/02a2c011bdab74c6fb3c75474d40b3052059d95df7e73351460c8588d963/h11-0.16.0.tar.gz", hash = "sha256:4e35b956cf45792e4caa5885e69fba00bdbc6ffafbfa020300e549b208ee5ff1", size = 101250, upload-time = "2025-04-24T03:35:25.427Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, -] - -[[package]] -name = "httpcore" -version = "1.0.9" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "certifi" }, - { name = "h11" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/06/94/82699a10bca87a5556c9c59b5963f2d039dbd239f25bc2a63907a05a14cb/httpcore-1.0.9.tar.gz", hash = "sha256:6e34463af53fd2ab5d807f399a9b45ea31c3dfa2276f15a2c3f00afff6e176e8", size = 85484, upload-time = "2025-04-24T22:06:22.219Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/7e/f5/f66802a942d491edb555dd61e3a9961140fd64c90bce1eafd741609d334d/httpcore-1.0.9-py3-none-any.whl", hash = "sha256:2d400746a40668fc9dec9810239072b40b4484b640a8c38fd654a024c7a1bf55", size = 78784, upload-time = "2025-04-24T22:06:20.566Z" }, -] - -[[package]] -name = "httpx" -version = "0.28.1" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "anyio" }, - { name = "certifi" }, - { name = "httpcore" }, - { name = "idna" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/b1/df/48c586a5fe32a0f01324ee087459e112ebb7224f646c0b5023f5e79e9956/httpx-0.28.1.tar.gz", hash = "sha256:75e98c5f16b0f35b567856f597f06ff2270a374470a5c2392242528e3e3e42fc", size = 141406, upload-time = "2024-12-06T15:37:23.222Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/2a/39/e50c7c3a983047577ee07d2a9e53faf5a69493943ec3f6a384bdc792deb2/httpx-0.28.1-py3-none-any.whl", hash = "sha256:d909fcccc110f8c7faf814ca82a9a4d816bc5a6dbfea25d6591d6985b8ba59ad", size = 73517, upload-time = "2024-12-06T15:37:21.509Z" }, -] - [[package]] name = "identify" version = "2.6.18"