test(coverage): batches 13–18 — Rust unit tests for 40+ modules (#530)#618
Merged
senamakel merged 8 commits intotinyhumansai:mainfrom Apr 17, 2026
Merged
Conversation
…inyhumansai#530) Add 36 new tests: - core/all: registry integrity (no duplicates, schema-controller parity), rpc_method_name, namespace_description, validate_params, schema lookup - proxy_config: parse_scope, parse_string_list (CSV, array, errors), parse_optional_string_update, env_snapshot, proxy_json, security gates All 4096 tests pass.
tinyhumansai#530) Add 31 new tests for helpers module: - vec_to_bytes/bytes_to_vec roundtrip, cosine_similarity (identical, orthogonal, mismatched, zero), collapse_whitespace, normalize_search_text, tokenize_search_terms, normalize_graph_entity/predicate, json_string_array, merge_unique_string_arrays, json_i64 (int/float/missing/string), recency_score (current/old/future), chunk_document_content All 4127 tests pass.
…igger history (tinyhumansai#530) Add 27 new tests across 4 modules: - accessibility/terminal: is_text_role, is_terminal_app, looks_like_terminal_buffer, extract_terminal_input_context, noise line detection - channels/runtime/supervision: compute_max_in_flight_messages clamping - security/detect: all sandbox backend fallback paths (landlock, firejail, bubblewrap, docker on non-linux), disabled-via-enabled-false - composio/trigger_history: list_recent with limit, empty store, field validation All 4154 tests pass.
…ice schemas + skills/types (tinyhumansai#530) Add 49 new tests across 6 modules: - learning/schemas: catalog, schema validation, unknown fallback - update/schemas: check/apply schemas, optional staging_dir, controller parity - encryption/schemas: encrypt/decrypt schemas, read_required helper - doctor/schemas: report/models schemas, read_optional helper (none/null/value/error) - service/schemas: 8 lifecycle functions, restart optional params, daemon_host - skills/types: ToolResult success/error/json/mixed, serde roundtrip, ToolContent All 4203 tests pass.
…umansai#530) Add tests for is_tab_key_down/is_escape_key_down (platform-safe), macOS-specific constant validation. All 4216 tests pass.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds extensive #[cfg(test)] unit tests across many modules (schemas, helpers, tools, accessibility, runtime, security, memory, service, etc.) validating registry invariants, schema/controller mappings, parsing helpers, and assorted utility behaviors. No production code or public API signatures were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/openhuman/accessibility/keys.rs (1)
128-133: Tautological assertion — low signal test.
kvk_constants_are_correctasserts the constants equal the same numeric literals they are defined with, so it can only fail if someone edits both the constant and the test in opposite directions. Consider either removing it or anchoring it to an external source of truth (e.g., Apple'sHIToolbox/Events.hkVK_Tab = 0x30,kVK_Escape = 0x35) via a comment, so the test guards against an actual regression rather than re-stating the definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/accessibility/keys.rs` around lines 128 - 133, The test kvk_constants_are_correct is tautological because it compares KVK_TAB and KVK_ESCAPE to the same numeric literals; instead either remove the test or make it assert against an external source of truth: change the expected values to Apple's official constants (kVK_Tab = 0x30, kVK_Escape = 0x35) and add a comment citing HIToolbox/Events.h as the source, or delete the kvk_constants_are_correct test entirely so the code isn't asserting its own definitions.src/openhuman/memory/store/unified/helpers.rs (1)
192-425: LGTM — well-structured test coverage for helper functions.Tests are deterministic, cover meaningful edge cases (empty vectors, orthogonal/zero/mismatched-length cosine inputs, JSON type-mismatch fallthrough, recency clamping for past/present/future, whitespace-only chunking), and correctly target the helpers' documented behavior. No production logic touched, so the logging guidelines don't apply here.
One small optional suggestion: consider adding a case for
cosine_similaritywith partially negative dot product (e.g.,[1.0, 0.0]vs[-1.0, 0.0]) to lock in theclamp(0.0, 1.0)lower-bound behavior, since that branch is currently not exercised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/memory/store/unified/helpers.rs` around lines 192 - 425, Add a small unit test to exercise the negative-dot-product branch for UnifiedMemory::cosine_similarity by asserting a case like vectors [1.0_f32, 0.0] vs [-1.0_f32, 0.0] yields the clamped lower-bound (0.0) to ensure the clamp(0.0, 1.0) behavior is covered; add it alongside the other cosine_similarity tests within the tests module so the behavior is deterministically verified.src/openhuman/channels/runtime/supervision.rs (1)
91-120: Tighten assertions and drop the duplicated zero-channel test.
compute_max_in_flight_messages_clamps_to_min(Lines 110-114) exercises the same input (0) ascompute_max_in_flight_messages_zero_channels(Lines 91-95) with a strictly weaker assertion, so it adds no coverage. Also, theone_channelandmany_channelscases can be asserted exactly (1×4=4 → clamped toCHANNEL_MIN_IN_FLIGHT_MESSAGES=8; 100×4=400 → clamped toCHANNEL_MAX_IN_FLIGHT_MESSAGES=64) — usingassert_eq!would catch regressions in the clamp constants or multiplier, whereas the current range-only assertion would silently pass if, e.g.,CHANNEL_PARALLELISM_PER_CHANNELchanged. Consider replacing the min-clamp test with a more meaningful boundary case (e.g., the exactchannel_countwhere the result transitions off the min, or just below the max).♻️ Suggested tightening
#[test] fn compute_max_in_flight_messages_one_channel() { let result = compute_max_in_flight_messages(1); - assert!(result >= CHANNEL_MIN_IN_FLIGHT_MESSAGES); - assert!(result <= CHANNEL_MAX_IN_FLIGHT_MESSAGES); + // 1 * CHANNEL_PARALLELISM_PER_CHANNEL (4) < CHANNEL_MIN_IN_FLIGHT_MESSAGES (8) → clamped up. + assert_eq!(result, CHANNEL_MIN_IN_FLIGHT_MESSAGES); } - #[test] - fn compute_max_in_flight_messages_clamps_to_min() { - let result = compute_max_in_flight_messages(0); - assert!(result >= CHANNEL_MIN_IN_FLIGHT_MESSAGES); - } - #[test] fn compute_max_in_flight_messages_clamps_to_max() { let result = compute_max_in_flight_messages(usize::MAX); - assert!(result <= CHANNEL_MAX_IN_FLIGHT_MESSAGES); + // saturating_mul overflow → still clamped to max. + assert_eq!(result, CHANNEL_MAX_IN_FLIGHT_MESSAGES); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/runtime/supervision.rs` around lines 91 - 120, Remove the duplicated zero-channel test and tighten the assertions: drop compute_max_in_flight_messages_clamps_to_min (which duplicates compute_max_in_flight_messages_zero_channels) and change compute_max_in_flight_messages_one_channel and compute_max_in_flight_messages_many_channels to assert exact values (using assert_eq!) based on the multiplier and clamp constants (compute_max_in_flight_messages(1) == CHANNEL_MIN_IN_FLIGHT_MESSAGES and compute_max_in_flight_messages(100) == CHANNEL_MAX_IN_FLIGHT_MESSAGES given CHANNEL_PARALLELISM_PER_CHANNEL and clamps); add a meaningful boundary test instead of the removed duplicate (e.g., a channel_count that yields the first value > CHANNEL_MIN_IN_FLIGHT_MESSAGES or a channel_count just below the value that clamps to CHANNEL_MAX_IN_FLIGHT_MESSAGES) to verify the transition off the min/max clamps.src/openhuman/security/detect.rs (2)
194-222: Weak assertions inbubblewrap_backend_falls_back_when_unavailableanddocker_backend_falls_back_when_unavailable.Both tests assert only
sandbox.is_available(), which istrueforNoopSandboxas well as for any real backend (pertraits.rs). As written, these tests pass regardless of whether the fallback path was actually exercised — they don't distinguish "Bubblewrap/Docker returned" from "fell back to Noop". Consider asserting onsandbox.name()with the expected set of outcomes so the fallback branch is actually verified on environments without the backend installed.♻️ Suggested tightening
let sandbox = create_sandbox(&config); - // Bubblewrap probably isn't installed on CI/dev — expect fallback - assert!(sandbox.is_available()); + // Bubblewrap usually isn't installed on CI/dev; accept either the real backend + // or the Noop fallback, but ensure the result is one of the known names. + let name = sandbox.name(); + assert!( + matches!(name, "none" | "bubblewrap"), + "unexpected sandbox name: {name}" + );And similarly for the Docker test (
"none" | "docker").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/security/detect.rs` around lines 194 - 222, The tests bubblewrap_backend_falls_back_when_unavailable and docker_backend_falls_back_when_unavailable currently only assert sandbox.is_available(), which doesn't distinguish a real backend from the Noop fallback; update each test (the SecurityConfig/SandboxConfig setup that uses SandboxBackend::Bubblewrap and SandboxBackend::Docker and the create_sandbox(...) call) to assert on sandbox.name() instead, checking that the returned name is one of the expected values (e.g., "none" or "bubblewrap" for the Bubblewrap test; "none" or "docker" for the Docker test) so the fallback path is actually verified rather than relying solely on is_available().
161-192: Landlock/Firejail non-Linux tests are no-ops on Linux runners.
landlock_backend_on_non_linux_falls_backandfirejail_backend_on_non_linux_falls_backwrap their onlyassert_eq!insideif std::env::consts::OS != "linux". On Linux CI (the common case), these tests execute zero assertions and always pass, providing no real coverage. Consider either gating the entire test with#[cfg(not(target_os = "linux"))]so it's clear the test is platform-specific, or adding a complementary assertion valid on Linux (e.g.,name()is one of{"none", "landlock"}/{"none", "firejail"}).♻️ Example
- #[test] - fn landlock_backend_on_non_linux_falls_back() { + #[cfg(not(target_os = "linux"))] + #[test] + fn landlock_backend_on_non_linux_falls_back() { ... let sandbox = create_sandbox(&config); - if std::env::consts::OS != "linux" { - assert_eq!(sandbox.name(), "none"); - } + assert_eq!(sandbox.name(), "none"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/security/detect.rs` around lines 161 - 192, The two tests landlock_backend_on_non_linux_falls_back and firejail_backend_on_non_linux_falls_back currently guard their assertions with if std::env::consts::OS != "linux", making them no-ops on Linux; update each test to either be platform-gated with #[cfg(not(target_os = "linux"))] so the whole test only runs on non-Linux, or expand the test to include a Linux branch that asserts create_sandbox(&config).name() returns an expected value for Linux (e.g., one of "none" or the backend-specific name) so the test performs a meaningful assertion on all platforms; locate these tests and the use of SecurityConfig/SandboxConfig/SandboxBackend and create_sandbox to apply the change.src/openhuman/composio/trigger_history.rs (1)
284-319: Optional: extract repeated test setup into a helper.The temp workspace + store setup is duplicated across tests (Line 284-288, Line 303-307, Line 314-318). A small helper would reduce noise and keep each test focused on assertions.
♻️ Suggested refactor
#[cfg(test)] mod tests { use super::*; + + fn make_store() -> ComposioTriggerHistoryStore { + let temp = tempfile::tempdir().expect("tempdir"); + let workspace = temp.path().join("workspace"); + fs::create_dir_all(&workspace).expect("workspace dir"); + ComposioTriggerHistoryStore::new(&workspace).expect("store") + } #[test] fn list_recent_with_limit_one() { - let temp = tempfile::tempdir().expect("tempdir"); - let workspace = temp.path().join("workspace"); - fs::create_dir_all(&workspace).expect("workspace dir"); - - let store = ComposioTriggerHistoryStore::new(&workspace).expect("store"); + let store = make_store(); // ... } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/composio/trigger_history.rs` around lines 284 - 319, Extract the repeated test setup (tempfile::tempdir, workspace path creation, fs::create_dir_all, and ComposioTriggerHistoryStore::new) into a small test helper function (e.g., fn make_test_store() -> ComposioTriggerHistoryStore or fn make_test_store() -> (tempfile::TempDir, ComposioTriggerHistoryStore) if you need to keep the TempDir alive) and call that helper from the tests that currently duplicate the setup (tests using ComposioTriggerHistoryStore::new in list_recent, list_recent_empty_store, and record_trigger_returns_entry_with_correct_fields); update those tests to use the helper instead of repeating the tempfile/workspace/store creation.src/openhuman/doctor/schemas.rs (1)
139-146: Assert length equality beforezipfor robust drift detection.
zipsilently truncates to the shorter iterator; ifall_controller_schemas()andall_registered_controllers()diverge in length, this test will pass. Match theencryption/servicestyle with an explicitassert_eq!(s.len(), c.len()).♻️ Proposed tweak
fn schemas_and_controllers_match() { let s = all_controller_schemas(); let c = all_registered_controllers(); + assert_eq!(s.len(), c.len()); for (schema, ctrl) in s.iter().zip(c.iter()) { assert_eq!(schema.function, ctrl.schema.function); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/doctor/schemas.rs` around lines 139 - 146, The test schemas_and_controllers_match should first assert the two collections have equal length to avoid silent truncation by zip: call assert_eq!(s.len(), c.len()) (or assert_eq!(s.len(), c.len(), "mismatch between all_controller_schemas and all_registered_controllers")) immediately after obtaining s = all_controller_schemas() and c = all_registered_controllers(), then proceed to zip and compare schema.function vs ctrl.schema.function as before; this ensures divergence in count fails the test.src/openhuman/update/schemas.rs (1)
169-177: Assert equal lengths beforezipto avoid silent pass on size mismatch.
ziptruncates to the shorter iterator, so a drift betweenall_controller_schemas()andall_registered_controllers()(e.g., someone adds only one side) would not fail this test. Mirror theencryption/servicetests which asserts.len() == c.len()first.♻️ Proposed tweak
fn schemas_and_controllers_match() { let s = all_controller_schemas(); let c = all_registered_controllers(); + assert_eq!(s.len(), c.len()); for (schema, ctrl) in s.iter().zip(c.iter()) { assert_eq!(schema.function, ctrl.schema.function); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/update/schemas.rs` around lines 169 - 177, The test schemas_and_controllers_match currently zips all_controller_schemas() and all_registered_controllers(), which can silently truncate mismatched collections; before iterating, add an assertion that s.len() == c.len() (or assert_eq!(s.len(), c.len(), "schemas and controllers count mismatch")) to fail when counts differ, then proceed to zip and compare schema.function to ctrl.schema.function; reference the test function name schemas_and_controllers_match and the helpers all_controller_schemas and all_registered_controllers when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/tools/impl/system/proxy_config.rs`:
- Around line 711-724: The test missing_action_returns_error is misleading and a
no-op because ProxyConfigTool::execute defaults a missing "action" to "get" (see
execute implementation dispatching to handle_get), so update the test to assert
the documented default behavior: rename the test (e.g.,
missing_action_defaults_to_get) and replace the match with an explicit assertion
that tool.execute(json!({})).await returns Ok and that the returned ToolResult
(or resulting state from handle_get) matches the expected "get" behavior;
alternatively remove the test if you prefer not to cover this case. Ensure you
update references to the test name and assert either Ok(_) or specific fields in
the returned result instead of discarding it.
---
Nitpick comments:
In `@src/openhuman/accessibility/keys.rs`:
- Around line 128-133: The test kvk_constants_are_correct is tautological
because it compares KVK_TAB and KVK_ESCAPE to the same numeric literals; instead
either remove the test or make it assert against an external source of truth:
change the expected values to Apple's official constants (kVK_Tab = 0x30,
kVK_Escape = 0x35) and add a comment citing HIToolbox/Events.h as the source, or
delete the kvk_constants_are_correct test entirely so the code isn't asserting
its own definitions.
In `@src/openhuman/channels/runtime/supervision.rs`:
- Around line 91-120: Remove the duplicated zero-channel test and tighten the
assertions: drop compute_max_in_flight_messages_clamps_to_min (which duplicates
compute_max_in_flight_messages_zero_channels) and change
compute_max_in_flight_messages_one_channel and
compute_max_in_flight_messages_many_channels to assert exact values (using
assert_eq!) based on the multiplier and clamp constants
(compute_max_in_flight_messages(1) == CHANNEL_MIN_IN_FLIGHT_MESSAGES and
compute_max_in_flight_messages(100) == CHANNEL_MAX_IN_FLIGHT_MESSAGES given
CHANNEL_PARALLELISM_PER_CHANNEL and clamps); add a meaningful boundary test
instead of the removed duplicate (e.g., a channel_count that yields the first
value > CHANNEL_MIN_IN_FLIGHT_MESSAGES or a channel_count just below the value
that clamps to CHANNEL_MAX_IN_FLIGHT_MESSAGES) to verify the transition off the
min/max clamps.
In `@src/openhuman/composio/trigger_history.rs`:
- Around line 284-319: Extract the repeated test setup (tempfile::tempdir,
workspace path creation, fs::create_dir_all, and
ComposioTriggerHistoryStore::new) into a small test helper function (e.g., fn
make_test_store() -> ComposioTriggerHistoryStore or fn make_test_store() ->
(tempfile::TempDir, ComposioTriggerHistoryStore) if you need to keep the TempDir
alive) and call that helper from the tests that currently duplicate the setup
(tests using ComposioTriggerHistoryStore::new in list_recent,
list_recent_empty_store, and record_trigger_returns_entry_with_correct_fields);
update those tests to use the helper instead of repeating the
tempfile/workspace/store creation.
In `@src/openhuman/doctor/schemas.rs`:
- Around line 139-146: The test schemas_and_controllers_match should first
assert the two collections have equal length to avoid silent truncation by zip:
call assert_eq!(s.len(), c.len()) (or assert_eq!(s.len(), c.len(), "mismatch
between all_controller_schemas and all_registered_controllers")) immediately
after obtaining s = all_controller_schemas() and c =
all_registered_controllers(), then proceed to zip and compare schema.function vs
ctrl.schema.function as before; this ensures divergence in count fails the test.
In `@src/openhuman/memory/store/unified/helpers.rs`:
- Around line 192-425: Add a small unit test to exercise the
negative-dot-product branch for UnifiedMemory::cosine_similarity by asserting a
case like vectors [1.0_f32, 0.0] vs [-1.0_f32, 0.0] yields the clamped
lower-bound (0.0) to ensure the clamp(0.0, 1.0) behavior is covered; add it
alongside the other cosine_similarity tests within the tests module so the
behavior is deterministically verified.
In `@src/openhuman/security/detect.rs`:
- Around line 194-222: The tests bubblewrap_backend_falls_back_when_unavailable
and docker_backend_falls_back_when_unavailable currently only assert
sandbox.is_available(), which doesn't distinguish a real backend from the Noop
fallback; update each test (the SecurityConfig/SandboxConfig setup that uses
SandboxBackend::Bubblewrap and SandboxBackend::Docker and the
create_sandbox(...) call) to assert on sandbox.name() instead, checking that the
returned name is one of the expected values (e.g., "none" or "bubblewrap" for
the Bubblewrap test; "none" or "docker" for the Docker test) so the fallback
path is actually verified rather than relying solely on is_available().
- Around line 161-192: The two tests landlock_backend_on_non_linux_falls_back
and firejail_backend_on_non_linux_falls_back currently guard their assertions
with if std::env::consts::OS != "linux", making them no-ops on Linux; update
each test to either be platform-gated with #[cfg(not(target_os = "linux"))] so
the whole test only runs on non-Linux, or expand the test to include a Linux
branch that asserts create_sandbox(&config).name() returns an expected value for
Linux (e.g., one of "none" or the backend-specific name) so the test performs a
meaningful assertion on all platforms; locate these tests and the use of
SecurityConfig/SandboxConfig/SandboxBackend and create_sandbox to apply the
change.
In `@src/openhuman/update/schemas.rs`:
- Around line 169-177: The test schemas_and_controllers_match currently zips
all_controller_schemas() and all_registered_controllers(), which can silently
truncate mismatched collections; before iterating, add an assertion that s.len()
== c.len() (or assert_eq!(s.len(), c.len(), "schemas and controllers count
mismatch")) to fail when counts differ, then proceed to zip and compare
schema.function to ctrl.schema.function; reference the test function name
schemas_and_controllers_match and the helpers all_controller_schemas and
all_registered_controllers when applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c83e4f6a-36c5-419c-ac8a-750c86392232
📒 Files selected for processing (14)
src/core/all.rssrc/openhuman/accessibility/keys.rssrc/openhuman/accessibility/terminal.rssrc/openhuman/channels/runtime/supervision.rssrc/openhuman/composio/trigger_history.rssrc/openhuman/doctor/schemas.rssrc/openhuman/encryption/schemas.rssrc/openhuman/learning/schemas.rssrc/openhuman/memory/store/unified/helpers.rssrc/openhuman/security/detect.rssrc/openhuman/service/schemas.rssrc/openhuman/skills/types.rssrc/openhuman/tools/impl/system/proxy_config.rssrc/openhuman/update/schemas.rs
Comment on lines
+711
to
+724
| #[tokio::test] | ||
| async fn missing_action_returns_error() { | ||
| let tmp = TempDir::new().unwrap(); | ||
| let tool = ProxyConfigTool::new(test_config(&tmp).await, test_security()); | ||
| let result = tool.execute(json!({})).await; | ||
| // Missing action may return Err or ToolResult::error | ||
| match result { | ||
| Err(_) => {} | ||
| Ok(r) => { | ||
| // Some implementations return success with help text; just verify it ran | ||
| let _ = r; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Misleading test name and no-op assertions.
Looking at execute (line 368–372), a missing action field defaults to "get", which successfully dispatches to handle_get. So missing_action_returns_error will never observe an error — the match arms both effectively pass unconditionally (the Ok arm discards the result with let _ = r;). This test doesn't verify any behavior and its name contradicts the actual contract.
Consider either renaming and asserting the documented default behavior, or removing the test:
♻️ Suggested rename + real assertion
- #[tokio::test]
- async fn missing_action_returns_error() {
- let tmp = TempDir::new().unwrap();
- let tool = ProxyConfigTool::new(test_config(&tmp).await, test_security());
- let result = tool.execute(json!({})).await;
- // Missing action may return Err or ToolResult::error
- match result {
- Err(_) => {}
- Ok(r) => {
- // Some implementations return success with help text; just verify it ran
- let _ = r;
- }
- }
- }
+ #[tokio::test]
+ async fn missing_action_defaults_to_get() {
+ let tmp = TempDir::new().unwrap();
+ let tool = ProxyConfigTool::new(test_config(&tmp).await, test_security());
+ let result = tool.execute(json!({})).await.unwrap();
+ assert!(!result.is_error, "{:?}", result.output());
+ assert!(result.output().contains("proxy"));
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/tools/impl/system/proxy_config.rs` around lines 711 - 724, The
test missing_action_returns_error is misleading and a no-op because
ProxyConfigTool::execute defaults a missing "action" to "get" (see execute
implementation dispatching to handle_get), so update the test to assert the
documented default behavior: rename the test (e.g.,
missing_action_defaults_to_get) and replace the match with an explicit assertion
that tool.execute(json!({})).await returns Ok and that the returned ToolResult
(or resulting state from handle_get) matches the expected "get" behavior;
alternatively remove the test if you prefer not to cover this case. Ensure you
update references to the test name and assert either Ok(_) or specific fields in
the returned result instead of discarding it.
…inyhumansai#530) Add 7 new tests: - service/restart: default source/reason, whitespace trimming, empty-string defaults, RestartStatus serde, startup delay noop - memory/store/factories: effective_memory_backend_name, migration-disabled error All 4223 tests pass.
…mas (tinyhumansai#530) Add 38 new tests across 2 modules: - tree_summarizer/schemas: catalog parity, all 5 functions, param helpers (read_required, read_optional, read_optional_timestamp), type_name, namespace_input - subconscious/schemas: catalog parity, all 10 functions, required input validation, field/field_req/field_opt helpers All 4261 tests pass.
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Continues the test coverage push from #607 (merged). Adds 486 new unit tests across 40+ modules, bringing total from 3,730 → 4,216.
Modules covered in this PR:
Test plan
cargo test --lib— all 4,216 tests passcargo fmt --all --check— cleanCloses #530 (partial — continues coverage push toward 80% target)
Summary by CodeRabbit