diff --git a/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock.md b/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock.md new file mode 100644 index 00000000..1c45ad9c --- /dev/null +++ b/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock.md @@ -0,0 +1,55 @@ +# FU-P11-T1-1: Refactor `_FakeWebUIConfig` to `MagicMock(spec=WebUIConfig)` + +**Status:** In Progress +**Branch:** feature/FU-P11-T1-1-refactor-fake-webuiconfig-mock +**Priority:** P3 +**Dependencies:** P11-T1 ✅ + +--- + +## Problem + +`tests/unit/test_main.py` defines a hand-rolled `_FakeWebUIConfig` inner class in two test methods: + +- `test_main_records_metrics_on_request_and_response` (line ~369) +- `test_main_does_not_record_metrics_when_request_has_no_method` (line ~467) + +Each stub manually declares `host`, `port`, `audit_log_dir`, `audit_max_file_size_mb`, `audit_max_files`, `audit_enabled`, and `audit_capture_payload`. Whenever a new property is added to the real `WebUIConfig`, every stub must be updated or the tests will not reflect the real interface. + +--- + +## Solution + +Replace both `_FakeWebUIConfig` inner classes with `MagicMock(spec=WebUIConfig)`, setting only the attributes actually needed by the production code path being exercised. The `spec=` constraint makes Mock raise `AttributeError` for attributes that do not exist on the real `WebUIConfig`, auto-enforcing the real interface. + +--- + +## Target File + +- `tests/unit/test_main.py` + +--- + +## Deliverables + +1. Remove both `_FakeWebUIConfig` inner class definitions. +2. Import `WebUIConfig` at the top of the test file (from `mcpbridge_wrapper.webui.config`). +3. Replace each `_FakeWebUIConfig` class usage in the `patch(...)` calls with a `MagicMock(spec=WebUIConfig)` instance that has the required attributes wired. +4. Validate all tests still pass. + +--- + +## Acceptance Criteria + +- [ ] No `_FakeWebUIConfig` class definition remains in `test_main.py` +- [ ] `WebUIConfig` imported at test module level +- [ ] Both `patch(..., _FakeWebUIConfig)` replaced with `patch(..., return_value=)` or `new_callable` pattern +- [ ] `pytest tests/unit/test_main.py` — all tests green +- [ ] `pytest --cov` — coverage remains ≥ 90% +- [ ] `ruff check src/` — no new lint errors + +--- + +## Notes + +The `_FakeWebUIConfig` is used as a replacement *class* (not instance) in `patch("mcpbridge_wrapper.webui.config.WebUIConfig", _FakeWebUIConfig)`. When the production code calls `WebUIConfig(config_path=...)`, it instantiates the fake. The refactored version should patch with a `MagicMock` that, when called, returns a configured mock instance. diff --git a/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Validation_Report.md b/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Validation_Report.md new file mode 100644 index 00000000..b1d78031 --- /dev/null +++ b/SPECS/ARCHIVE/FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/FU-P11-T1-1_Validation_Report.md @@ -0,0 +1,39 @@ +# Validation Report: FU-P11-T1-1 + +**Task:** Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` +**Date:** 2026-02-16 +**Branch:** feature/FU-P11-T1-1-refactor-fake-webuiconfig-mock +**Verdict:** PASS + +--- + +## Changes Made + +**File:** `tests/unit/test_main.py` + +1. Added import: `from mcpbridge_wrapper.webui.config import WebUIConfig` +2. Replaced `_FakeWebUIConfig` inner class in `test_main_records_metrics_for_tracked_request_and_response` with: + - `fake_webui_config = MagicMock(spec=WebUIConfig)` with required attributes set + - `mock_webui_config_cls = MagicMock(spec=WebUIConfig, return_value=fake_webui_config)` as the patched class +3. Replaced `_FakeWebUIConfig` inner class in `test_main_does_not_record_metrics_when_request_has_no_method` with equivalent `MagicMock(spec=WebUIConfig)` pattern. + +--- + +## Quality Gates + +| Gate | Result | +|------|--------| +| `pytest tests/unit/test_main.py` | ✅ 22 passed | +| `pytest` (full suite) | ✅ 465 passed, 5 skipped | +| `pytest --cov` (≥90%) | ✅ 95.6% | +| `ruff check src/` | ✅ All checks passed | + +--- + +## Acceptance Criteria Verification + +- [x] No hand-rolled `_FakeWebUIConfig` class remains in `test_main.py` +- [x] All existing `test_main.py` tests pass (22/22) +- [x] `pytest` suite remains green (465 passed) +- [x] `WebUIConfig` imported at module level in test file +- [x] Both patch sites use `MagicMock(spec=WebUIConfig)` pattern diff --git a/SPECS/ARCHIVE/INDEX.md b/SPECS/ARCHIVE/INDEX.md index 56f61a24..b5d824b7 100644 --- a/SPECS/ARCHIVE/INDEX.md +++ b/SPECS/ARCHIVE/INDEX.md @@ -1,6 +1,6 @@ # mcpbridge-wrapper Tasks Archive -**Last Updated:** 2026-02-16 (FU-P11-T2-2) +**Last Updated:** 2026-02-16 (FU-P11-T1-1) ## Archived Tasks @@ -100,6 +100,7 @@ | P12-T2 | [P12-T2_Add_Tool_Parameter_Frequency_Analysis/](P12-T2_Add_Tool_Parameter_Frequency_Analysis/) | 2026-02-16 | PASS | | FU-P11-T2-1 | [FU-P11-T2-1_Push_Session_Data_via_WebSocket/](FU-P11-T2-1_Push_Session_Data_via_WebSocket/) | 2026-02-16 | PASS | | FU-P11-T2-2 | [FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/](FU-P11-T2-2_Add_limit_query_param_to_GET_api_sessions/) | 2026-02-16 | PASS | +| FU-P11-T1-1 | [FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/](FU-P11-T1-1_Refactor_FakeWebUIConfig_to_MagicMock/) | 2026-02-16 | PASS | ## Historical Artifacts @@ -163,6 +164,7 @@ | [REVIEW_P12-T4_data_storage_documentation.md](P12-T4_Add_documentation_about_data_storage/REVIEW_P12-T4_data_storage_documentation.md) | Review report for P12-T4 | | [REVIEW_P12-T2_param_frequency_analysis.md](P12-T2_Add_Tool_Parameter_Frequency_Analysis/REVIEW_P12-T2_param_frequency_analysis.md) | Review report for P12-T2 | | [REVIEW_FU-P11-T2-2_limit_query_param.md](_Historical/REVIEW_FU-P11-T2-2_limit_query_param.md) | Review report for FU-P11-T2-2 | +| [REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md](_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md) | Review report for FU-P11-T1-1 | ## Archive Log @@ -280,3 +282,5 @@ | 2026-02-16 | FU-P11-T2-1 | Archived REVIEW_FU-P11-T2-1_session_websocket_push report | | 2026-02-16 | FU-P11-T2-2 | Archived Add_limit_query_param_to_GET_api_sessions (PASS) | | 2026-02-16 | FU-P11-T2-2 | Archived REVIEW_FU-P11-T2-2_limit_query_param report | +| 2026-02-16 | FU-P11-T1-1 | Archived Refactor_FakeWebUIConfig_to_MagicMock (PASS) | +| 2026-02-16 | FU-P11-T1-1 | Archived REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor report | diff --git a/SPECS/ARCHIVE/_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md new file mode 100644 index 00000000..7323a5c3 --- /dev/null +++ b/SPECS/ARCHIVE/_Historical/REVIEW_FU-P11-T1-1_fake_webuiconfig_refactor.md @@ -0,0 +1,57 @@ +## REVIEW REPORT — FU-P11-T1-1: Refactor _FakeWebUIConfig to MagicMock(spec=WebUIConfig) + +**Scope:** origin/main..HEAD +**Files:** 1 (tests/unit/test_main.py) +**Date:** 2026-02-16 + +--- + +### Summary Verdict +- [ ] Approve +- [x] Approve with comments +- [ ] Request changes +- [ ] Block + +--- + +### Critical Issues + +None. + +--- + +### Secondary Issues + +**[Low] Two near-identical mock setup blocks rather than a shared helper** + +Both `test_main_records_metrics_for_tracked_request_and_response` and `test_main_does_not_record_metrics_when_request_has_no_method` now contain identical 9-line mock setup sequences (`fake_webui_config*` + `mock_webui_config_cls*`). A shared helper function or pytest fixture would reduce duplication, but this is a style concern and not a correctness issue. + +Suggested fix (optional): Extract a module-level or class-level `_make_fake_webui_config()` factory that returns `(mock_cls, mock_instance)`. + +--- + +### Architectural Notes + +- Using `MagicMock(spec=WebUIConfig)` correctly enforces the real interface: any future access to a property that does not exist on `WebUIConfig` will raise `AttributeError`, catching regressions early. +- The mock class (`mock_webui_config_cls`) is patched as the class replacement so that `WebUIConfig(config_path=...)` calls in production code return the pre-configured mock instance. This pattern is idiomatic and sound. +- `spec=WebUIConfig` on the *class* mock (not just the instance mock) provides double protection: the class-level spec prevents accidental calls to non-existent class methods, while the instance spec covers attribute access. + +--- + +### Tests + +- All 22 tests in `test_main.py` pass. +- Full suite: 465 passed, 5 skipped. +- Coverage: 95.6% (requirement: ≥ 90%). +- No regressions introduced. + +--- + +### Next Steps + +- Optional (Low): Extract shared `_make_fake_webui_config()` factory to avoid duplicated setup in two tests. No new task needed unless the pattern grows to a third usage. +- No blockers or required follow-ups identified. + +--- + +**FOLLOW-UP decision:** No actionable findings that warrant new workplan tasks. FOLLOW-UP step is skipped per FLOW rules. diff --git a/SPECS/INPROGRESS/next.md b/SPECS/INPROGRESS/next.md index 3fe14cd6..c419fe90 100644 --- a/SPECS/INPROGRESS/next.md +++ b/SPECS/INPROGRESS/next.md @@ -4,15 +4,15 @@ The previously selected task has been archived. ## Recently Archived +- 2026-02-16 — FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` (PASS) - 2026-02-16 — FU-P11-T2-2: Add `limit` query param to `GET /api/sessions` (PASS) - 2026-02-16 — FU-P11-T2-1: Push session data via WebSocket (PASS) - 2026-02-16 — P12-T2: Add Tool Parameter Frequency Analysis (PASS) - 2026-02-15 — P11-T4: Add Keyboard Shortcuts & Command Palette (PASS) -- 2026-02-15 — P11-T1: Add Tool Call Detail Inspector (PASS) ## Suggested Next Tasks -- FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` (P3, depends on P11-T1 ✅) - FU-P12-T2-1: Fix stacking click event listeners in `updateLatencyTable` (P3, depends on P12-T2 ✅) - FU-P12-T1-1: Remove or document `MCPInitializeParams` in schemas (P3, depends on P12-T1 ✅) - FU-P12-T3-2: Add `error_code` column to audit CSV export (P3, depends on P12-T3 ✅) +- FU-P12-T1-2: Add code comment clarifying stdin-only client capture in `on_request` (P3, depends on P12-T1 ✅) diff --git a/SPECS/Workplan.md b/SPECS/Workplan.md index 0c1d445d..eb0052f9 100644 --- a/SPECS/Workplan.md +++ b/SPECS/Workplan.md @@ -2007,7 +2007,7 @@ Phase 9 Follow-up Backlog --- -#### FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` +#### ✅ FU-P11-T1-1: Refactor `_FakeWebUIConfig` test stub to use `MagicMock(spec=WebUIConfig)` - **Description:** The hand-rolled `_FakeWebUIConfig` class in `tests/unit/test_main.py` must be manually updated every time a new property is added to `WebUIConfig`. Refactor it to use `MagicMock(spec=WebUIConfig)` with only the properties needed by the test wired up, so the spec auto-enforces the real interface and future property additions do not break the test. - **Priority:** P3 - **Dependencies:** P11-T1 diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index ec1ee70c..2924e6d4 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -5,6 +5,7 @@ from unittest.mock import MagicMock, patch from mcpbridge_wrapper.__main__ import main +from mcpbridge_wrapper.webui.config import WebUIConfig class TestMain: @@ -366,16 +367,15 @@ def _capture_forwarder(_bridge, on_request=None): mock_stdin_forwarder.side_effect = _capture_forwarder # Patch WebUI components so --web-ui does not start any real threads/servers. - class _FakeWebUIConfig: - def __init__(self, config_path=None): - self._data = {} - self.host = "127.0.0.1" - self.port = 8080 - self.audit_log_dir = "/tmp" - self.audit_max_file_size_mb = 1 - self.audit_max_files = 1 - self.audit_enabled = False - self.audit_capture_payload = False + fake_webui_config = MagicMock(spec=WebUIConfig) + fake_webui_config.host = "127.0.0.1" + fake_webui_config.port = 8080 + fake_webui_config.audit_log_dir = "/tmp" + fake_webui_config.audit_max_file_size_mb = 1 + fake_webui_config.audit_max_files = 1 + fake_webui_config.audit_enabled = False + fake_webui_config.audit_capture_payload = False + mock_webui_config_cls = MagicMock(spec=WebUIConfig, return_value=fake_webui_config) class _TriggeringQueue: def __init__(self, on_first_get): @@ -400,7 +400,7 @@ def get(self): return_value=MagicMock(), ), patch( "mcpbridge_wrapper.webui.config.WebUIConfig", - _FakeWebUIConfig, + mock_webui_config_cls, ), patch( "mcpbridge_wrapper.webui.server.run_server_in_thread", return_value=MagicMock(), @@ -464,16 +464,15 @@ def _capture_forwarder(_bridge, on_request=None): mock_stdin_forwarder.side_effect = _capture_forwarder - class _FakeWebUIConfig: - def __init__(self, config_path=None): - self._data = {} - self.host = "127.0.0.1" - self.port = 8080 - self.audit_log_dir = "/tmp" - self.audit_max_file_size_mb = 1 - self.audit_max_files = 1 - self.audit_enabled = False - self.audit_capture_payload = False + fake_webui_config2 = MagicMock(spec=WebUIConfig) + fake_webui_config2.host = "127.0.0.1" + fake_webui_config2.port = 8080 + fake_webui_config2.audit_log_dir = "/tmp" + fake_webui_config2.audit_max_file_size_mb = 1 + fake_webui_config2.audit_max_files = 1 + fake_webui_config2.audit_enabled = False + fake_webui_config2.audit_capture_payload = False + mock_webui_config_cls2 = MagicMock(spec=WebUIConfig, return_value=fake_webui_config2) with patch( "mcpbridge_wrapper.webui.shared_metrics.SharedMetricsStore", @@ -483,7 +482,7 @@ def __init__(self, config_path=None): return_value=MagicMock(), ), patch( "mcpbridge_wrapper.webui.config.WebUIConfig", - _FakeWebUIConfig, + mock_webui_config_cls2, ), patch( "mcpbridge_wrapper.webui.server.run_server_in_thread", return_value=MagicMock(),