Skip to content

Turn-orchestrator tests: shared harness + retire approval-overlap cases#193

Closed
ytallo wants to merge 2 commits into
refactor/turn-orchestrator-cleanupfrom
refactor/turn-orchestrator-test-trim
Closed

Turn-orchestrator tests: shared harness + retire approval-overlap cases#193
ytallo wants to merge 2 commits into
refactor/turn-orchestrator-cleanupfrom
refactor/turn-orchestrator-test-trim

Conversation

@ytallo
Copy link
Copy Markdown
Contributor

@ytallo ytallo commented May 27, 2026

Summary

Test-side follow-up to #185. Consolidates duplicated mock harnesses and retires tests already covered end-to-end by the new parallel-approval e2e. Production code is untouched.

Stacked on refactor/turn-orchestrator-cleanup (#185) — review/merge after that one.

Cuts

  • Shared message-builder helper (_helpers/builders.ts): a single makeAssistant / makeAssistantWithCalls / agentTriggerCall replaces three near-identical inline builders across functions.test.ts, function-execute.test.ts, and function-awaiting-approval.test.ts.
  • Shared fakeIii helper (_helpers/fakeIii.ts): the canonical { iii, calls } fake used by assistant.test.ts, provisioning.test.ts, hook.test.ts, run-start.test.ts, preflight.test.ts, steering.test.ts, and run-transition.test.ts. Responder option covers both static-map and per-call-callback shapes.
  • Shared state-store fake (_helpers/stateStoreIii.ts): the richer fake (in-memory state::get / state::set / state::update + turn::* wake capture) used by on-record-written.e2e.test.ts — was a 60-line inline implementation.
  • Retired tautological cases in state.test.ts: three tests that just restated the newRecord builder defaults. The invariant validators below them stay — those test real error paths.
  • Retired overlap in function-awaiting-approval.test.ts: dropped the handleAwaitingApproval allow→steering_check and undecided-parking cases — both are exercised end-to-end by tests/integration/parallel-approval.e2e.test.ts (with the dedicated harness from Turn-orchestrator: leaner durable FSM (reactive approval, finishSession teardown, readability) #185). Kept the function_execute resume case (no e2e counterpart) and the applyDecisionToPrepared unit tests.

What's left untouched

  • provider-stream.test.ts, store.test.ts, the per-state stubPorts helpers — each is genuinely specialized (channel logic, emit capture, distinct Ports types). Consolidating them would add complexity without a real win.
  • parallel-approval-harness.ts and parallel-approval.e2e.test.ts — the reference implementation everything else now leans on.

Test plan

  • pnpm -C harness exec vitest run — full suite green (937 tests; -5 vs. main, all retired duplicates/tautologies)
  • pnpm -C harness typecheck — clean
  • biome 2.4.10 formatting applied
  • CI green

ytallo added 2 commits May 27, 2026 08:56
- _helpers/builders.ts: single makeAssistant / makeAssistantWithCalls /
  agentTriggerCall replaces three near-identical inline builders.
- _helpers/fakeIii.ts: shared { iii, calls } fake used by
  assistant/provisioning/hook/run-start tests; responder option covers
  both static maps and per-call callbacks.
- state.test.ts: drop three builder-restatement tests; the invariant
  parsers below cover real validation.

939 tests passing locally (-3 vs main: the retired tautologies).
… tests through shared fakes

- _helpers/stateStoreIii.ts: richer state-store-backed fake used by
  on-record-written.e2e; captures every state::get / state::set /
  state::update + turn::* wake.
- on-record-written.e2e.test.ts: switch to the shared helper.
- function-awaiting-approval.test.ts: drop the allow→steering_check
  and undecided-parking cases (parallel-approval.e2e covers both
  end-to-end with its dedicated harness); keep the function_execute
  resume case which the e2e doesn't exercise. Route the local makeIii
  through the shared fakeIii responder.
- preflight.test.ts, steering.test.ts, run-transition.test.ts: drop
  ad-hoc trigger fakes in favour of fakeIii({ responder }).

937 tests passing (-5 vs main: 3 tautological state.test.ts cases
from the previous commit + the 2 retired here).
@vercel
Copy link
Copy Markdown

vercel Bot commented May 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
workers Ready Ready Preview, Comment May 27, 2026 12:05pm

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e2b2869f-e3ee-49b3-ae8b-59ae830d797a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/turn-orchestrator-test-trim

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

skill-check — worker

0 verified, 13 skipped (no docs/).

Layer Result
structure
vale
ai

Three for three. Nicely done.

@ytallo ytallo closed this May 27, 2026
@ytallo ytallo deleted the refactor/turn-orchestrator-test-trim branch May 27, 2026 12:14
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