Skip to content

test: fix tests broken by PR #2918 SDK abstraction + add Copilot routing unit tests#2920

Merged
rysweet merged 1 commit intofeat/2917-power-steering-sdk-abstractionfrom
fix/2918-sdk-test-fixes
Mar 7, 2026
Merged

test: fix tests broken by PR #2918 SDK abstraction + add Copilot routing unit tests#2920
rysweet merged 1 commit intofeat/2917-power-steering-sdk-abstractionfrom
fix/2918-sdk-test-fixes

Conversation

@rysweet
Copy link
Owner

@rysweet rysweet commented Mar 7, 2026

Summary

Validation of PR #2918 (feat: power-steering SDK abstraction). Found and fixed test regressions introduced by the queryquery_llm API change, added comprehensive mock-based unit tests for the new Copilot routing, and confirmed philosophy compliance.

Changes

Test Fixes (broken by PR #2918)

  • test_power_steering_shutdown.py: Remove stale @patch("claude_power_steering.query") decorator from test_no_timing_regression_during_normal_operation. The test only calls is_shutting_down() — no SDK mock needed.

  • test_issue_1872_bug_fixes.py: Update 8 tests to patch claude_power_steering.query_llm (AsyncMock returning str) instead of the removed claude_power_steering.query. Also update call_args[1]["prompt"]call_args[0][0] for positional arg access.

Type Fix (philosophy compliance)

  • power_steering_sdk.py (both copies): Fix _detector_cache type annotation from object | None to str | None and remove the # type: ignore[return-value] workaround.

New Mock-Based Unit Tests

  • tests/test_power_steering_sdk.py — 21 tests covering:
    • _query_copilot async lifecycle: await client.start(), create_session(), send_and_wait(), stop() all awaited
    • client.stop() called even when send_and_wait raises (finally block)
    • stop() exceptions suppressed
    • Text extraction from event.data.content (and None/missing data cases)
    • query_llm routing: copilot launcher → Copilot SDK, claude launcher → Claude SDK
    • Fallback: copilot launcher + no Copilot SDK → Claude
    • Fallback: claude launcher + no Claude SDK → Copilot
    • Neither SDK available → returns ""
    • SDK_AVAILABLE exported, query_llm in __all__
    • _query_claude text extraction from content blocks and string content

Philosophy Compliance

power_steering_sdk.py: MINOR_ISSUES / Overall COMPLIANT

  • All core principles pass (simplicity, brick philosophy, async patterns, fail-open, zero-BS, clear exports)
  • Minor: sys.path.insert coupling in _detect_launcher is fragile but protected by broad except

Test Results

All 68 key tests pass:

Suite Result
test_power_steering_sdk.py (new) 21/21
test_copilot_e2e_power_steering.py 22/22
test_sdk_integration.py 5/5
test_power_steering_shutdown.py 20/20 (was failing)

Validates all PR #2918 success criteria.

🤖 Generated with Claude Code

…ing unit tests

PR #2918 replaced `from claude_agent_sdk import query` with `from
power_steering_sdk import query_llm`, removing `query` as a module-level
attribute from `claude_power_steering`. This broke two test files that
patched the now-removed attribute.

Fixes:
- test_power_steering_shutdown.py: remove stale @patch("claude_power_steering.query")
  decorator and unused mock_query param from test_no_timing_regression (test only
  calls is_shutting_down() which needs no SDK mock)
- test_issue_1872_bug_fixes.py: update 8 tests to patch
  `claude_power_steering.query_llm` (AsyncMock returning str) instead of
  the removed `claude_power_steering.query` (generator yielding MockMessage)

Bug fixes:
- power_steering_sdk.py (both copies): fix _detector_cache type annotation
  from `object | None` to `str | None` and remove the # type: ignore[return-value]
  comment that worked around the self-inflicted mismatch

New tests:
- tests/test_power_steering_sdk.py: 21 mock-based unit tests covering
  _query_copilot async lifecycle (start/create_session/send_and_wait/stop),
  event.data.content text extraction, query_llm routing to Copilot/Claude,
  fallback behavior, SDK_AVAILABLE flag, and _query_claude text extraction

All 68 tests across test_power_steering_sdk, test_sdk_integration,
test_copilot_e2e_power_steering, and test_power_steering_shutdown pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

Repo Guardian - Passed

All files in this PR are legitimate production code and tests. No ephemeral content detected.

Files analyzed:

  • .claude/tools/amplihack/hooks/power_steering_sdk.py - SDK abstraction layer
  • .claude/tools/amplihack/hooks/tests/test_issue_1872_bug_fixes.py - Bug fix test suite
  • .claude/tools/amplihack/hooks/tests/test_power_steering_sdk.py - SDK unit tests
  • .claude/tools/amplihack/hooks/tests/test_power_steering_shutdown.py - Shutdown behavior tests
  • amplifier-bundle/tools/amplihack/hooks/power_steering_sdk.py - SDK copy

All files are durable, reusable components that belong in the repository.

AI generated by Repo Guardian

@rysweet rysweet merged commit e9fa347 into feat/2917-power-steering-sdk-abstraction Mar 7, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant