refactor: extract shared test utilities into internal/testutil (#531)

[nextlevelshit]

Summary

Extracts reusable test infrastructure into a dedicated internal/testutil package.

Changes

• EventCollector: Thread-safe event collector for capturing progress events in tests
• MockStateStore: Shared mock implementation of state.Store interface used across test suites
• Test utilities: Centralized test helpers to reduce duplication

Benefits

• Reduces test code duplication across pipeline, relay, and integration tests
• Makes test infrastructure reusable and maintainable
• Enables consistent event collection patterns

Test Coverage

All tests passing with race detector:

go test -race ./...

Tests for all 32+ packages pass successfully.

[nextlevelshit]

Code Review (Wave Pipeline)

Verdict: APPROVE ✅

This PR cleanly extracts duplicated test infrastructure (EventCollector, MockStateStore, CreateTestManifest) into a shared internal/testutil package, removing ~560 lines of duplication across 10 test files. All tests pass. No production code is touched.

Security

No issues found. This is test-only code with no impact on security posture — no SQL, no HTTP handlers, no secrets, no deserialization of untrusted input.

Suggested Improvements

These are not blocking but would strengthen the PR:

1. Remove dead field savePipelineStateLocked (MEDIUM)

internal/testutil/statestore.go:19 — The savePipelineStateLocked bool field is declared but never read. SavePipelineState bypasses locking when an override is set regardless of this flag. Remove it to avoid confusion about the locking contract.

2. Add compile-time interface assertion (MEDIUM)

internal/testutil/statestore.go — MockStateStore implements 44 methods of state.StateStore with no compile-time check. Add:

var _ state.StateStore = (*MockStateStore)(nil)

This catches interface drift immediately rather than at test runtime.

3. Document limited WithXxx option coverage (MEDIUM)

internal/testutil/statestore.go — Only 9 of 44 methods have WithXxx functional options. The remaining 35 methods cannot be overridden from outside the package. At minimum, document which methods are overridable. Consider adding options for commonly needed methods as usage grows.

4. Consider adding testutil package tests (LOW)

The package has no direct tests — functionality is only exercised transitively through internal/pipeline tests. Direct tests for EventCollector concurrency and MockStateStore defaults would catch regressions closer to the source.

5. Confirm specs/ documents are intentional (LOW)

The PR includes specs/531-shared-test-utilities/{spec,plan,tasks}.md. If these are transient planning artifacts, consider removing before merge.

Positive Observations

• Thread safety done right: EventCollector uses sync.Mutex correctly with copy-on-read semantics. MockStateStore uses sync.RWMutex. Both are safe for concurrent test use.
• Silently fixes a pre-existing race: The old testEmitter in composition_test.go had no mutex — replacing it with the thread-safe EventCollector fixes a latent race condition.
• Clean migration: All old type references are fully removed. No leftover dead code in the migrated files.
• Functional options pattern: MockStateStore uses idiomatic Go functional options for test customization, making tests readable and composable.

---

Generated by Wave pr-review pipeline