refactor(test): remove all unittest.mock usage from Robot Framework integration tests #703

Closed
freemo wants to merge 5 commits from refactor/m3-remove-unittest-mock-integration into master
Owner

Summary

Removes all unittest.mock usage (MagicMock, patch, create_autospec, PropertyMock) from 41 Robot Framework integration test files, replacing them with real dependency instantiation via a new shared stub library.

Closes #699

Changes

Created robot/_testing_stubs.py (383 lines) providing lightweight, Pyright-clean test doubles for integration tests:

  • Stub — generic attribute-bag replacing MagicMock (returns Stub() for any missing attribute)
  • SpyStub — records calls for assertion without mocking behavior
  • patch_attr — context manager replacing unittest.mock.patch for temporary attribute overrides
  • fake_settings — builds a real Settings instance with test defaults, replacing MagicMock(spec=Settings)

Updated 41 files across three categories:

Category Files Change
Helper files using MagicMock and/or patch 31 Replaced with Stub, SpyStub, patch_attr, fake_settings from _testing_stubs
Helper files using only patch 7 Replaced unittest.mock.patch with patch_attr from _testing_stubs
Robot files with inline unittest.mock 3 (binding_resolution.robot, google_provider.robot, openai_provider.robot) Replaced inline MagicMock with Stub via helper keywords

No Behave unit tests were modified — they continue using unittest.mock as permitted by CONTRIBUTING.md.

Quality Checks

  • nox -e typecheck — 0 Pyright errors
  • nox -e lint — all checks passed
  • nox -e format — all files unchanged
  • nox -e integration_tests — all affected Robot suites pass
  • No unittest.mock imports remain in any robot/ file
## Summary Removes all `unittest.mock` usage (`MagicMock`, `patch`, `create_autospec`, `PropertyMock`) from 41 Robot Framework integration test files, replacing them with real dependency instantiation via a new shared stub library. Closes #699 ## Changes Created `robot/_testing_stubs.py` (383 lines) providing lightweight, Pyright-clean test doubles for integration tests: - **`Stub`** — generic attribute-bag replacing `MagicMock` (returns `Stub()` for any missing attribute) - **`SpyStub`** — records calls for assertion without mocking behavior - **`patch_attr`** — context manager replacing `unittest.mock.patch` for temporary attribute overrides - **`fake_settings`** — builds a real `Settings` instance with test defaults, replacing `MagicMock(spec=Settings)` Updated 41 files across three categories: | Category | Files | Change | |----------|-------|--------| | Helper files using MagicMock and/or patch | 31 | Replaced with `Stub`, `SpyStub`, `patch_attr`, `fake_settings` from `_testing_stubs` | | Helper files using only patch | 7 | Replaced `unittest.mock.patch` with `patch_attr` from `_testing_stubs` | | Robot files with inline unittest.mock | 3 (`binding_resolution.robot`, `google_provider.robot`, `openai_provider.robot`) | Replaced inline `MagicMock` with `Stub` via helper keywords | No Behave unit tests were modified — they continue using `unittest.mock` as permitted by CONTRIBUTING.md. ## Quality Checks - `nox -e typecheck` — 0 Pyright errors - `nox -e lint` — all checks passed - `nox -e format` — all files unchanged - `nox -e integration_tests` — all affected Robot suites pass - No `unittest.mock` imports remain in any `robot/` file
refactor(test): remove all unittest.mock usage from Robot Framework integration tests
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 17s
CI / security (pull_request) Successful in 38s
CI / typecheck (pull_request) Successful in 39s
CI / unit_tests (pull_request) Successful in 3m5s
CI / integration_tests (pull_request) Successful in 3m42s
CI / docker (pull_request) Successful in 59s
CI / coverage (pull_request) Successful in 7m15s
CI / benchmark-regression (pull_request) Successful in 36m40s
1067ed5e40
Replace MagicMock, patch, create_autospec, and PropertyMock with custom
lightweight test doubles in robot/_testing_stubs.py:

- Stub: drop-in for MagicMock with call tracking, return_value,
  side_effect, context-manager and iteration protocol support
- SpyStub: wraps a real object while tracking calls
- patch_attr(): replaces unittest.mock.patch/patch.object with
  context-manager and start/stop API support
- fake_settings(): replaces create_autospec(Settings, instance=True)
- _CallRecord: lightweight replacement for unittest.mock.call

All 42 files in robot/ directory updated. No file in robot/ imports
from unittest.mock. Features/mocks directory left untouched.

Passes: nox -e lint, nox -e typecheck (0 errors), nox -e unit_tests
(372 features, 10553 scenarios, 0 failed).

Closes #699
freemo added this to the v3.2.0 milestone 2026-03-12 00:23:53 +00:00
refactor: replace all unittest.mock patterns with real dependencies in robot integration tests
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / quality (pull_request) Successful in 18s
CI / build (pull_request) Successful in 18s
CI / security (pull_request) Successful in 35s
CI / typecheck (pull_request) Successful in 41s
CI / unit_tests (pull_request) Successful in 3m5s
CI / integration_tests (pull_request) Failing after 3m27s
CI / docker (pull_request) Successful in 53s
CI / coverage (pull_request) Successful in 5m31s
CI / benchmark-regression (pull_request) Successful in 35m29s
30b6bfc810
- Delete _testing_stubs.py (382 lines of Stub/SpyStub/patch_attr/fake_settings)
- Create _test_container.py with real service factories backed by in-memory SQLite
- Replace all .return_value/.assert_called/.call_args patterns across 41 helper files
- Use real PlanLifecycleService, CorrectionService, DecisionService, InvariantService
- Use real ResourceRegistryService, NamespacedProjectRepository, ProjectResourceLinkRepository
- Use real ActorService + ActorRegistry for TDD actor validation tests
- Use types.SimpleNamespace for lightweight structural interfaces (DI container resolve)
- Use InMemoryChangeSetStore (production class) for changeset tests
- Tag LLM-dependent tests with [Tags] llm-required for CI skip
- All modified files pass ruff check, ruff format, and pyright with zero errors

Implements CONTRIBUTING.md line 568: no mocking in integration tests.
- Add create_test_decision_service() and create_plan_for_decisions() to
  _test_container.py so DecisionService shares the same in-memory SQLite
  DB as PlanLifecycleService (fixes FK constraint on decisions.plan_id)
- Fix helper_plan_correct_tree_wiring.py: use shared DB for decisions
- Fix helper_event_bus.py: use shared DB for decision_service_emits_event
- Fix helper_m4_e2e_verification.py: use shared DB for cli-plan-tree,
  add regex-based JSON extraction to handle interleaved log lines
- Fix helper_resource_handlers.py: use real temp dirs with
  SandboxStrategy.NONE for git-resolve and fs-resolve tests
- Fix helper_m1_sourcecode_smoke.py: plan_diff() now creates a real
  PlanApplyService with InMemoryChangeSetStore and changeset_id
- Fix helper_actor_cli_show.py: upsert() instead of save(), flexible
  list assertion for built-in actors
- Fix helper_apply_pipeline.py: set terminal APPLIED state for
  already-applied test
- Fix helper_cli_extensions.py: set inputs_schema on action object
  (not via create_action kwarg)
- Fix helper_m3_decision_validation_smoke.py: dict access for
  attach_validation() return value
- Fix helper_plan_cli_spec.py: project_links with ProjectLink objects
- Add prepare_plan_for_execute/apply calls across lifecycle helpers

All 110 targeted robot tests pass. Quality gates: typecheck (0 errors),
lint (all passed), format (1415 files formatted).
fix(test): use model_id instead of model in provider attribute checks
Some checks failed
CI / lint (pull_request) Successful in 15s
CI / typecheck (pull_request) Successful in 40s
CI / security (pull_request) Successful in 38s
CI / quality (pull_request) Successful in 34s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 19s
CI / unit_tests (pull_request) Successful in 5m12s
CI / integration_tests (pull_request) Successful in 6m7s
CI / docker (pull_request) Successful in 4m12s
CI / coverage (pull_request) Successful in 6m54s
CI / benchmark-regression (pull_request) Has been cancelled
72ddf83fdf
GoogleChatProvider and OpenAIChatProvider store the model name as
model_id (inherited from LangChainChatProvider), not model. The
attribute check tests were asserting the wrong attribute name.
fix(test): fix IndentationError in provider env-gating inside Robot Catenate blocks
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 17s
CI / typecheck (pull_request) Successful in 38s
CI / security (pull_request) Successful in 40s
CI / integration_tests (pull_request) Successful in 3m28s
CI / unit_tests (pull_request) Successful in 5m17s
CI / docker (pull_request) Successful in 22s
CI / coverage (pull_request) Successful in 5m35s
CI / benchmark-regression (pull_request) Successful in 35m54s
1a584563a7
Robot Framework's Catenate with SEPARATOR=\n strips leading whitespace
from continuation lines, so a multi-line if/body block:

    ...    if not api_key:
    ...        print('SKIP: ...'); sys.exit(0)

becomes invalid Python (print at column 0 after the if). Collapse the
if and its body onto a single line so indentation is not needed.
freemo self-assigned this 2026-03-12 20:33:27 +00:00
Author
Owner

PM Status — Day 32

Status: CONFLICTED — needs rebase. Part of the mock removal initiative.

PR: Remove all unittest.mock from Robot Framework integration tests (41 files). M3 (v3.2.0), overdue. Author: @freemo. 0 comments, 0 reviews.

Context: This is part of the mock removal epic (#699). Issues #698 and #700 were already completed via direct commits to master. #724 was also completed on master. This PR covers the remaining 41-file integration test cleanup.

Action Required:

  1. @freemo: Rebase onto current master. Since you've been pushing mock removal commits directly to master, some of this PR's changes may already be on master — verify after rebase.
  2. Labels: Has Type/Task. Missing Priority (High), MoSCoW (Must have), Points (13), State (In Progress).
  3. Needs peer review after rebase — suggest @brent.edwards since mock removal affects test infrastructure.
## PM Status — Day 32 **Status**: CONFLICTED — needs rebase. Part of the mock removal initiative. **PR**: Remove all `unittest.mock` from Robot Framework integration tests (41 files). M3 (v3.2.0), overdue. Author: @freemo. 0 comments, 0 reviews. **Context**: This is part of the mock removal epic (#699). Issues #698 and #700 were already completed via direct commits to master. #724 was also completed on master. This PR covers the remaining 41-file integration test cleanup. **Action Required**: 1. **@freemo**: Rebase onto current master. Since you've been pushing mock removal commits directly to master, some of this PR's changes may already be on master — verify after rebase. 2. Labels: Has Type/Task. Missing Priority (High), MoSCoW (Must have), Points (13), State (In Progress). 3. Needs peer review after rebase — suggest @brent.edwards since mock removal affects test infrastructure.
Author
Owner

Rebase Required

@freemo — This PR has merge conflicts with master. Please rebase onto the latest master and force-push. See also: #668, #669, #708, #713, #720, #722 (all need rebase).

## Rebase Required @freemo — This PR has merge conflicts with `master`. Please rebase onto the latest `master` and force-push. See also: #668, #669, #708, #713, #720, #722 (all need rebase).
Author
Owner

PM Review — Day 34

Status: NOT mergeable (conflicts), 0 reviews, M3 (v3.2.0), Priority/High, 21 points
Closes: #699 | Author: @freemo

Review Summary

Architecturally sound refactor that replaces all unittest.mock usage in Robot Framework integration tests with real service dependencies backed by in-memory SQLite. This is the correct direction per CONTRIBUTING.md §568 (integration tests must exercise real services).

42 files changed across robot/ — the largest scope PR currently open.

Key Findings

The refactor exposed and fixed real bugs hidden by mocks:

  • FK constraint violations on decisions.plan_id (masked by MagicMock)
  • Actor CLI calling .save() (doesn't exist — real method is .upsert())
  • Provider tests checking .model (real attribute is .model_id)
  • Plan lifecycle missing required state transitions
  • project_links expecting raw dicts instead of ProjectLink objects

This validates the entire approach — mocks were silently accepting wrong attribute names, wrong method calls, and wrong data structures.

Issues

[BLOCKING] Merge conflictsmergeable: false. Several overlapping mock-removal issues (#698, #700, #724) were landed via direct master commits, causing conflicts. Rebase required.

[STALE] PR description misleading — The body describes the Phase 1 _testing_stubs.py approach (Stub, SpyStub, patch_attr), which was deleted in commit 2. The actual implementation uses _test_container.py with real services. Description must be updated after rebase.

[MINOR] Messy commit history — Commit 1 creates _testing_stubs.py, commit 2 deletes it entirely. Consider squashing during rebase.

[MINOR] Points discrepancy — PR is labeled Points/21 but parent issue #699 is Points/5. The scope justifies 21.

Action Items

Who Action Deadline
@freemo Rebase onto current master, resolve conflicts, update PR description Day 36
@freemo Verify all 110 robot tests pass post-rebase Day 36
@brent.edwards Peer review after rebase (QA lead, test infrastructure domain) Day 37

This PR is high-value — it eliminates an entire class of false-positive test results across the integration suite. Priority to unblock after rebase.

## PM Review — Day 34 **Status**: NOT mergeable (conflicts), 0 reviews, M3 (v3.2.0), Priority/High, 21 points **Closes**: #699 | **Author**: @freemo ### Review Summary Architecturally sound refactor that replaces all `unittest.mock` usage in Robot Framework integration tests with real service dependencies backed by in-memory SQLite. This is the correct direction per CONTRIBUTING.md §568 (integration tests must exercise real services). **42 files changed** across `robot/` — the largest scope PR currently open. ### Key Findings **The refactor exposed and fixed real bugs hidden by mocks:** - FK constraint violations on `decisions.plan_id` (masked by MagicMock) - Actor CLI calling `.save()` (doesn't exist — real method is `.upsert()`) - Provider tests checking `.model` (real attribute is `.model_id`) - Plan lifecycle missing required state transitions - `project_links` expecting raw dicts instead of `ProjectLink` objects This validates the entire approach — mocks were silently accepting wrong attribute names, wrong method calls, and wrong data structures. ### Issues **[BLOCKING] Merge conflicts** — `mergeable: false`. Several overlapping mock-removal issues (#698, #700, #724) were landed via direct master commits, causing conflicts. Rebase required. **[STALE] PR description misleading** — The body describes the Phase 1 `_testing_stubs.py` approach (Stub, SpyStub, patch_attr), which was **deleted** in commit 2. The actual implementation uses `_test_container.py` with real services. Description must be updated after rebase. **[MINOR] Messy commit history** — Commit 1 creates `_testing_stubs.py`, commit 2 deletes it entirely. Consider squashing during rebase. **[MINOR] Points discrepancy** — PR is labeled Points/21 but parent issue #699 is Points/5. The scope justifies 21. ### Action Items | Who | Action | Deadline | |-----|--------|----------| | @freemo | **Rebase** onto current master, resolve conflicts, update PR description | Day 36 | | @freemo | Verify all 110 robot tests pass post-rebase | Day 36 | | @brent.edwards | **Peer review** after rebase (QA lead, test infrastructure domain) | Day 37 | This PR is high-value — it eliminates an entire class of false-positive test results across the integration suite. Priority to unblock after rebase.
freemo left a comment

PM Status — Day 34

@freemo — unittest.mock removal from 41 Robot files (#699). Has merge conflicts. Must Have / Priority High.

Status: Conflicted. This is the largest remaining mock removal item. Depends on PR #710 (CI LLM keys) being configured first.

Dependency chain: PR #710 (CI keys) → PR #703 (mock removal) → M3 mock removal 100% complete.

Priority: Critical for M3 closure. Please rebase after #710 is ready.


PM status — Day 34

## PM Status — Day 34 @freemo — unittest.mock removal from 41 Robot files (#699). Has merge conflicts. **Must Have / Priority High**. **Status**: Conflicted. This is the largest remaining mock removal item. Depends on PR #710 (CI LLM keys) being configured first. **Dependency chain**: PR #710 (CI keys) → PR #703 (mock removal) → M3 mock removal 100% complete. **Priority**: Critical for M3 closure. Please rebase after #710 is ready. --- *PM status — Day 34*
Author
Owner

wont do, did E2E tests instead

wont do, did E2E tests instead
freemo closed this pull request 2026-03-14 22:21:24 +00:00
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
Required
Details
CI / build (pull_request) Successful in 16s
Required
Details
CI / quality (pull_request) Successful in 17s
Required
Details
CI / typecheck (pull_request) Successful in 38s
Required
Details
CI / security (pull_request) Successful in 40s
Required
Details
CI / integration_tests (pull_request) Successful in 3m28s
Required
Details
CI / unit_tests (pull_request) Successful in 5m17s
Required
Details
CI / docker (pull_request) Successful in 22s
Required
Details
CI / coverage (pull_request) Successful in 5m35s
Required
Details
CI / benchmark-regression (pull_request) Successful in 35m54s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!703
No description provided.