Skip to content

test(backends): add engine contract test for all registered engines#837

Merged
aaight merged 2 commits intodevfrom
feature/engine-contract-test
Mar 14, 2026
Merged

test(backends): add engine contract test for all registered engines#837
aaight merged 2 commits intodevfrom
feature/engine-contract-test

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Mar 14, 2026

Summary

  • Adds tests/unit/backends/engine-contract.test.ts — a contract test that validates all 4 built-in engines against the AgentEngine interface
  • Iterates over every registered engine using describe.each for consistent per-engine validation
  • Validates getEngineCatalog() returns definitions for all registered engines

What is tested

  • registerBuiltInEngines() successfully registers all 4 engines: llmist, claude-code, codex, opencode
  • Each engine's definition has the required fields: id, label, description, capabilities (array), modelSelection, logLabel
  • Each engine's definition.id matches its registry key
  • execute is a function on each engine
  • supportsAgentType() is a function that returns a boolean for known agent types (implementation, review, splitting)
  • Optional resolveModel is a function if present (validates Story chore: update llmist and zangief dependencies #3 compliance)
  • Optional beforeExecute/afterExecute are functions if present (validates Story feat: add UpdateChecklistItem gadget and improve agent workflow #5 compliance)
  • getEngineCatalog() returns definitions for all registered engines and references the same objects as the registry

Test plan

  • tests/unit/backends/engine-contract.test.ts exists and passes (49 tests)
  • npm run typecheck passes
  • npm run lint passes

Card: https://trello.com/c/69b57342a440fa1ff9a93706

🕵️ claude-code · claude-sonnet-4-6 · run details

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Clean, well-structured contract test that validates all 4 built-in engines against the AgentEngine interface. One duplicated test doesn't verify what its name claims.

Should Fix

tests/unit/backends/engine-contract.test.ts, lines 25-30 — The test "registers exactly the expected engines" is functionally identical to "registers all 4 built-in engines" (lines 17-22). Both iterate EXPECTED_ENGINE_IDS and assert each is contained in registeredIds:

// Test 1 (line 17): "registers all 4 built-in engines"
const registeredIds = getRegisteredEngines();
for (const id of EXPECTED_ENGINE_IDS) {
    expect(registeredIds).toContain(id);
}

// Test 2 (line 25): "registers exactly the expected engines"
const registeredIds = getRegisteredEngines();
for (const id of EXPECTED_ENGINE_IDS) {
    expect(registeredIds).toContain(id);
}

The name "exactly" implies it should also verify no unexpected engines are registered, but it doesn't. To make this test meaningful, add an assertion like:

expect(registeredIds).toHaveLength(EXPECTED_ENGINE_IDS.length);

Or remove the duplicate entirely.

🕵️ claude-code · claude-opus-4-6 · run details

…gful

Add toHaveLength assertion so the test verifies no unexpected engines
are registered, not just that the expected ones are present.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — Clean contract test that validates all 4 built-in engines against the AgentEngine interface. Test isolation is sound (vitest uses pool: 'forks', so the module-level registry state won't leak across test files). The describe.each pattern keeps per-engine validation DRY, optional member checks correctly guard with \!== undefined, and the hardcoded EXPECTED_ENGINE_IDS array will catch newly added engines via the length assertion. CI green.

🕵️ claude-code · claude-opus-4-6 · run details

@aaight aaight merged commit 48a87de into dev Mar 14, 2026
6 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.

2 participants