test: add unit tests for zero-coverage crates/core modules#424
Conversation
Add #[cfg(test)] mod tests blocks to registry.rs, node.rs, pins.rs, control.rs, helpers.rs, view_data.rs, and metrics.rs — covering all public APIs that previously had zero test coverage. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![staging-devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/870844?s=60&v=4){kind=small}
| Shutdown, | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
📝 Info: PR is test-only and leaves production paths unchanged
All hunks in this PR add #[cfg(test)] modules below existing production definitions; no non-test function, struct, enum, or constant bodies were changed. That means the existing production behaviors for control serialization, config parsing, batching, output sending, pin matching, registry lookup/resource loading, metric boundaries, and view-data emission are unaffected by this PR.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| #[test] | ||
| fn with_resource_manager() { | ||
| let rm = Arc::new(ResourceManager::new(ResourcePolicy::default())); | ||
| let reg = NodeRegistry::with_resource_manager(rm); | ||
| assert!(reg.definitions().is_empty()); | ||
| } |
There was a problem hiding this comment.
🚩 Resource registry tests do not exercise resource-backed node creation
The new registry tests cover static/dynamic registration, descriptions, overwrite behavior, synchronous create_node, and with_resource_manager, but they do not add coverage for register_static_with_resource, register_dynamic_with_resource, or create_node_async resource-cache behavior in crates/core/src/registry.rs:238-365. This is not a regression because those paths were not modified by the PR, but it is a remaining coverage gap if the goal is comprehensive registry coverage.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
There was a problem hiding this comment.
Addressed — added tests for register_static_with_resource, register_dynamic_with_resource, and create_node_async (success, unknown kind, no-resource-manager fallback) in the latest push.
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
📝 Info: Comment-heavy tests may conflict with the repository’s preference but are not a correctness issue
The repository guidance discourages comments that restate obvious code, and several touched files already contain substantial API documentation and the new tests add descriptive names rather than new production comments. I did not report this as a bug because comments/documentation style issues are not severe and the added test code mostly relies on test names for intent; any cleanup would be editorial rather than behavioral.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Cover register_static_with_resource, register_dynamic_with_resource, and create_node_async (success, unknown kind, no-resource-manager fallback). Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Rename batch_packets_greedy_single_packet to _drains_one_extra_packet - Replace mechanical engine-control construct/destructure tests with Debug snapshot assertions - Add async OutputSender::send tests (direct + routed, success + error paths) - Add parse_config_optional invalid-type fallback test - Add parse_config_with_context happy path test - Add NodeRegistry::set_resource_manager test - Verify routed packet payload in output_sender_try_send_routed_success - Strengthen output_send_error_display_messages assertions with quoted names Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
2 participants
Summary
Adds comprehensive unit tests to 7
crates/core/modules that previously had zero test coverage:registry.rs,node.rs,pins.rs,control.rs,helpers.rs,view_data.rs, andmetrics.rs. Total of 152 tests in the crate (up from 80).Changes:
create_node_async,set_resource_manager, serialization round-tripsOutputSendersynctry_send+ asyncsendfor Direct and Routed routing (success, pin-not-found, channel-full, channel-closed), payload verification through routing, pin name caching,InitContextfield access,PipelineModedefaultsNodeControlMessageserde round-trips,ConnectionModedefault/serde/snake_case,EngineControlMessageDebug snapshot assertionsInputPin/OutputPinconstruction,PinCardinality::One/Broadcast/Dynamicmatching, equality, cloningparse_config_optional(valid, none, partial, invalid-type fallback),parse_config_required(valid, none, invalid),parse_config_with_context(missing, invalid, valid),batch_packets_greedy(drains extra, empty, batch-size limit),DEFAULT_BATCH_CAPACITYReview & Testing Checklist for Human
cargo test -p streamkit-core— all 152 tests should passcontrol.rstests are meaningful (check that variant names and field values appear in output)parse_config_optional_with_invalid_type_falls_back_to_defaultmatches the documented behavior athelpers.rs:24Notes
types.rs,packet_meta.rs,error.rs#[cfg(test)] mod tests { ... }blocks at the bottom of each file#[tokio::test]Link to Devin session: https://staging.itsdev.in/sessions/8a13dc1e2cee46288ace08340f796a6b
Requested by: @streamer45
Devin Review
f062e67(HEAD is93b896a)