Repository navigation
test(reborn): add phase 1 integration coverage - #3076
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive integration tests for the RuntimeDispatcher and process dispatching services. The new tests cover scenarios such as authorized request routing, handling missing runtime backends, registry validation, background process completion, and process termination logic. Feedback was provided to improve the accuracy of the dispatch_error_for_runtime test helper by passing the actual capability ID instead of using a hardcoded value, which enhances the reusability of the helper.
| fn dispatch_error_for_runtime( | ||
| runtime: RuntimeKind, | ||
| kind: RuntimeDispatchErrorKind, | ||
| ) -> DispatchError { | ||
| match runtime { | ||
| RuntimeKind::Wasm => DispatchError::Wasm { kind }, | ||
| RuntimeKind::Script => DispatchError::Script { kind }, | ||
| RuntimeKind::Mcp => DispatchError::Mcp { kind }, | ||
| RuntimeKind::FirstParty | RuntimeKind::System => DispatchError::UnsupportedRuntime { | ||
| capability: CapabilityId::new("system.unsupported").unwrap(), | ||
| runtime, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
The dispatch_error_for_runtime helper function uses a hardcoded capability ID system.unsupported when creating a DispatchError::UnsupportedRuntime. This could be misleading if this helper is used in more tests.
The RecordingAdapter has access to the actual capability_id from the RuntimeAdapterRequest. It would be better to pass this to dispatch_error_for_runtime and use it to construct the error, making the test helper more accurate and reusable. This aligns with the repository practice of providing semantically correct and clear error messages.
Note: You will also need to update the call sites in RecordingAdapter::dispatch_json to pass request.capability_id.
| fn dispatch_error_for_runtime( | |
| runtime: RuntimeKind, | |
| kind: RuntimeDispatchErrorKind, | |
| ) -> DispatchError { | |
| match runtime { | |
| RuntimeKind::Wasm => DispatchError::Wasm { kind }, | |
| RuntimeKind::Script => DispatchError::Script { kind }, | |
| RuntimeKind::Mcp => DispatchError::Mcp { kind }, | |
| RuntimeKind::FirstParty | RuntimeKind::System => DispatchError::UnsupportedRuntime { | |
| capability: CapabilityId::new("system.unsupported").unwrap(), | |
| runtime, | |
| }, | |
| } | |
| } | |
| fn dispatch_error_for_runtime( | |
| runtime: RuntimeKind, | |
| kind: RuntimeDispatchErrorKind, | |
| capability_id: &CapabilityId, | |
| ) -> DispatchError { | |
| match runtime { | |
| RuntimeKind::Wasm => DispatchError::Wasm { kind }, | |
| RuntimeKind::Script => DispatchError::Script { kind }, | |
| RuntimeKind::Mcp => DispatchError::Mcp { kind }, | |
| RuntimeKind::FirstParty | RuntimeKind::System => DispatchError::UnsupportedRuntime { | |
| capability: capability_id.clone(), | |
| runtime, | |
| }, | |
| } | |
| } |
References
- Create specific error variants for different failure modes to provide semantically correct and clear error messages.
* test(reborn): add phase 1 integration coverage * test(reborn): stabilize process kill integration test
Summary
Adds Phase 1 Reborn caller-level integration coverage from #3067 now that
ironclaw_processes(#3017) andironclaw_dispatcher(#3023) have landed onreborn-integration.This PR intentionally adds tests only; no production behavior changes.
Coverage
crates/ironclaw_dispatcher/tests/runtime_dispatcher_integration.rsCapabilityDispatchertrait objectcrates/ironclaw_processes/tests/process_dispatch_integration.rsProcessServices+ProcessHostEventingProcessStorekilltransitions toKilled, signals cancellation, blocks late executor success from overwriting terminal state, and suppresses misleading completion eventsVerification
CARGO_TARGET_DIR=/tmp/ironclaw-reborn-integration-target \ cargo test -p ironclaw_dispatcher -p ironclaw_processes CARGO_TARGET_DIR=/tmp/ironclaw-reborn-integration-target \ cargo clippy -q -p ironclaw_dispatcher -p ironclaw_processes --all-targets -- -D warnings cargo fmt --check git diff --checkRefs #3067.