Skip to content

test: memory status B1 regression + sync_helpers coverage (12 new tests) - #19

Merged
FZ2000 merged 4 commits into
mainfrom
test/status-and-sync-helpers
Mar 7, 2026
Merged

test: memory status B1 regression + sync_helpers coverage (12 new tests)#19
FZ2000 merged 4 commits into
mainfrom
test/status-and-sync-helpers

Conversation

@FZ2000

@FZ2000 FZ2000 commented Mar 7, 2026

Copy link
Copy Markdown
Owner

Coverage for the two gaps identified in the Forge audit: memory path in _tool_sync_status (B1 regression) and zero coverage on sync_helpers.

New: tests/test_manifest.py — 3 memory status tests

Added to TestToolSyncStatus:

Test What it catches
test_synced_when_memory_file_present The B1 crash — old code called .get() on strings from .values(). This would AttributeError before the fix in PR #18.
test_out_of_sync_when_memory_file_deleted Memory file recorded in manifest, then deleted → "out of sync"
test_synced_when_memory_not_yet_recorded Manifest has only MCP servers, memory dict is {} → no false "out of sync"

The memory tests failed against main (confirming they catch the real bug) and pass after cherry-picking the fix from fix/bugs.

New: tests/test_sync_helpers.py — 12 tests

TestResolveTargetTools (5 tests)

  • --tools flag parsing + whitespace strip
  • Empty string returns []
  • --all calls detect_installed_tools
  • --all with no detected tools returns []

TestSyncAll (6 tests)

  • Happy path returns True
  • All three phases called per tool (apply_skills, apply_mcp_servers, apply_memory_via_llm)
  • --no-memory skips apply_memory_via_llm
  • Partial failure (1 of 2 tools errors) → True (any_success)
  • All fail → False
  • Single tool success

TestSyncSkillsPerToolCounter (1 test)

  • Verifies per-tool success message shows 3 copied not 6 copied for 2 tools × 3 skills. Catches the B2 counter bug directly.

Depends on

Cherry-picks fix/bugs (e49818b) so the memory tests pass.
Intended to be merged after PR #18 lands on main.

Test count

137 passing (122 from PR #18 base + 15 new).

@FZ2000
FZ2000 merged commit a366722 into main Mar 7, 2026
4 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